[GRASS5] R_open_driver() after fork()

[Glynn Clements]

Radim Blazek wrote:

> > OK; the patch works as intended, then.
> >
> > In which case, should this be changed for 5.0.x? I'm not sure that it
> > should. While it does technically fix a bug, it's a bug that shouldn't
> > be triggered in normal use; the situation (where a client tries to
> > connect to a monitor while it is in use by another client) shouldn't
> > happen.
> 
> I think that if GUI (TclTk) is used, it may happen often. Question is 
> if in that case better is when module crashes or waits until the
> second (first) one finishes.
> Raster lib is probably stable now and no changes are expected in 5.0
> so we could add patched io.c to 5.1 (where the problem appeares).

Well, you can *make* the problem appear on 5.0.x, e.g. by running an
interactive display command in the background, then running another
display command.

The point is that sync_driver() is clearly intended to time out
eventually, but the code relies upon signal() having the SysV
semantics (which isn't the case for Linux/glibc2, which is probably
~90% of GRASS' user base).

> > > > More generally, fork() and high-level APIs don't mix.
> > >
> > > Yes, I see, it is old story - better framework for GUI.
> >
> > I wasn't referring to GRASS APIs in particular; many third-party
> > libraries have highly "undefined" behaviour w.r.t. fork().
> 
> DBMI drivers are also started by fork()+execl(), may it be a problem?

Well, I consider a native Win32 port to be desirable, so *all* use of
fork() is problematic in that sense.

Give some consideration to redesigning the DBMI driver interface to
use e.g. dlopen() (LoadLibrary() on Windows); most modern OSes have
equivalent functionality (besides dlopen(), XFree86 4.x includes code
for dynamically loading a.out, ELF and COFF files).

> I found for example, that it is impossible to start and close more drivers
> in 'FILO' order, driver opened as last must be closed first. 
> Is it bug in library? Somewhere in db_shutdown_driver (driver):?
> /* wait for the driver to finish */
>     status = -1;
>     while ((pid = wait(&status)) > 0 && pid != driver->pid) {}

It looks like the same problem; when you spawn the second driver, it
will inherit from the parent the descriptors for the client's end of
the connection to the first driver. The first driver won't receive EOF
until *all* copies of the other end of the pipe have been closed, i.e. 
until the parent closes its copy *and* the second driver is dead.

Again, one solution would be to set the close-on-exec flag on the
descriptors, in db_start_driver().

Basically, when spawning a subprocess, you normally want to close all
descriptors except those which are actually meant to be inherited. Of
course, that means that the code which performs the fork() must
actually be capable of enumerating the set of open descriptors. This
is why fork() and high-level APIs don't mix.

Using the close-on-exec flag only works if the library which allocates
the descriptor sets it; and there isn't a close-on-fork flag, so
fork-without-exec is problematic.

-- 
Glynn Clements <glynn.clements at virgin.net>