Re: closing extra fd's in gnome_execute_*



On Mon, 11 Jan 1999, Elliot Lee wrote:

> On Mon, 11 Jan 1999, Jeff Garzik wrote:
> 
> > On Mon, 11 Jan 1999, Manish Vachharajani wrote:
> > > Consider this.  I am running program foo.  foo opens a 30 megabyte file.
> > > While the file is open it runs gnome_execute_async, say for the
> > > help-browser or something.  Now my disk runs out of space, so I rm the 30
> > > meg file after quitting application foo.  Unfortunately, I won't regain
> > > that space until I close the help-browser.  If foo happens to start many
> > > programs, I have to quit them all before the space will be freed.  This
> > > will definitely bite us in the ass as well.  We really should close the
> > > fd's unless we want other behavior.
> > 
> > Some programs even use magic fd numbers to facilitate fd passing between
> > programs aware of these magic numbers. 
> > 
> > The default should be to close the fds; to do otherwise would violate
> > the Principle of Least Surprise...
> 
> You can't possibly close all the fd's without a pretty drastic performance
> hit, especially since the kernel is getting large FD set support (4
> billion FD's IIRC).

Use the sysconf routine with _SC_MAXOPENFD or something similar.  It is a
POSIX function so it should be ok.

> Programs that are broken need to be fixed, instead of putting tons of
> workarounds into other programs that use the broken ones. It's nice to
> talk about how you need it to work now with existing programs etc. etc.,
> but adding this into gnome-libs is short term "fix my particular problem
> right now" myopia, without looking at the long term, where all these
> broken programs will either get fixed or stop being used, and people will
> regret adding the behaviour that must remain for posterity.

You can't fix the problem of inheriting file descriptors for large files.
If one gets inherited, you CANNOT recover the space until all apps that
use that file are closed.  It is a little hard to tell which apps those
might be when fd's get inherited all over the place.

> The right way to get rid of extra fd's passed to child processes is to set
> the FD_CLOEXEC flag on the fd in the application, so that it doesn't stay
> open in the executed program. ORBit already does this for its fd's, and it
> should be done for all fd's except pipes.

Hmm, this is a good idea.  This is probably the correct fix then.

> The principle of least surprise implies (to me) doing nothing unless it is
> specifically asked for. If you want to put your "close fd's 0-N, and I'll
> pretend they all got closed" hack in libgnome, it would be better to add
> gnome_execute_*_closing_fds() variants, rather than changing the default.

For now the stuff closes fd's but I agree that we should set the CLOEXEC
flag on the fd's.  When I get a chance I will start putting that in.
Until then I think we really need to close the fd's since I have been
bitten both by the too many open fd's and the can't recover space problem.
It is damn annoying.  Reminds me of windows :)

> The default routines need to fit as wide a variety of cases as possible,
> with convenience wrappers done as special-case versions of the default
> routines.

I agree.

> The funny thing is I don't have any problems at all with bash and
> high-numbered fd's.

I still cannot track down the problem in bash, but I get an error of an
input buffer already existing for fd's when running configure.  It only
happens when bash is run from a process that comes from the session
manager, which opens a healthy number of fds.  It is also intermittent,
damn hard to track down.

> -- Elliot
> "In film you will find four basic story lines. Man versus man, man
>  versus nature, nature versus nature, and dog versus vampire."
>     - Steven Spielberg
> 

Manish Vachharajani               Some Haiku: A crash reduces
<mvachhar@vger.rutgers.edu>                   your expensive computer
                                              to a simple stone - Unknown



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]