A common bug on Posix seems to be to forget to close file descriptors not meant to stay open in child processes as this search reveals:

http://www.google.de/search?q=%22set+FD_CLOEXEC%22+OR+%22FD_CLOEXEC+not+set%22+bug

Unlike on Windows where this is opt-in, it is opt-out and you have to use FD_CLOEXEC, which closes the file descriptor across an "execve" (used to execute another program).

My proposal is here to add this flag to all file descriptors Phobos opens (files, sockets, streams) and make it a convention for Phobos development on Posix. The assumption is that this is the saner default nowadays, kudos to Windows here.

This issue came up in the std.process thread, where hacks around it for the sake of OS independent behavior are discussed.

Would this be feasible and work ?

-- 
Marco

I've taken a look around at how other cross-platform languages go about this...

OpenJDK:

> All file descriptors that are opened in the JVM and not specifically destined for a subprocess should have the close-on-exec flag set.  If we don't set it, then careless 3rd party native code might fork and exec without closing all appropriate file descriptors (e.g. as we do in closeDescriptors in UNIXProcess.c), and this in turn might:
>
> - cause end-of-file to fail to be detected on some file
>   descriptors, resulting in mysterious hangs, or
>
> - might cause an fopen in the subprocess to fail on a system
>   suffering from bug 1085341.
>
> (Yes, the default setting of the close-on-exec flag is a Unix
> design flaw)

Python:

> def _mkstemp_inner(dir, pre, suf, flags):
>     """Code common to mkstemp, TemporaryFile, and NamedTemporaryFile."""
> 
>     names = _get_candidate_names()
> 
>     for seq in range(TMP_MAX):
>         name = next(names)
>         file = _os.path.join(dir, pre + name + suf)
>         try:
>             fd = _os.open(file, flags, 0o600)
>             _set_cloexec(fd)
>             return (fd, _os.path.abspath(file))
>         except FileExistsError:
>             continue    # try again
> 
>     raise FileExistsError("No usable temporary file name found")

Ruby:

> Ruby sets close-on-exec flags of all file descriptors by default since
> Ruby 2.0.0. So you don’t need to set by yourself. Also, unsetting a
> close-on-exec flag can cause file descriptor leak if another thread
> use fork() and exec() (via system() method for example).
> If you really needs file descriptor inheritance to child process,
> use spawn()‘s argument such as fd=>fd.

Haskell:

I found some discussion here in 2009: http://therning.org/magnus/archives/727 I think they just rely on the C library at the moment for their System.IO, which in turn doesn't set FD_CLOEXEC. But I'm really bad at reading Haskell :p

Rust:

Uses C stdlib.

-----------------------------------------------------

From this small set of languages it looks like those that have their own IO implementation and don't rely on the C lib, mostly set FD_CLOEXEC by default.

Despite the increased maintenance cost I think we should adapt that behavoir in D as well.

-- 
Marco

On Monday, 11 March 2013 at 05:24:49 UTC, Marco Leise wrote:
>
> Python:
>
>> def _mkstemp_inner(dir, pre, suf, flags):
>>     """Code common to mkstemp, TemporaryFile, and NamedTemporaryFile."""

So it only sets the flag on temporary files?

> I think they just rely on the C library at the moment for their System.IO,
> which in turn doesn't set FD_CLOEXEC.

> Rust:
>
> Uses C stdlib.

Is there any discussion on why libc doesn't do it, and what do APIs that wrap the C API do?

> Despite the increased maintenance cost I think we should adapt that
> behavoir in D as well.

I don't think there would be a maintenance cost to speak of. Wouldn't it be a one-line addition to a few places?

Am Mon, 11 Mar 2013 06:32:42 +0100
schrieb "Vladimir Panteleev" <vladimir@thecybershadow.net>:

> On Monday, 11 March 2013 at 05:24:49 UTC, Marco Leise wrote:
> >
> > Python:
> >
> >> def _mkstemp_inner(dir, pre, suf, flags):
> >>     """Code common to mkstemp, TemporaryFile, and
> >> NamedTemporaryFile."""
> 
> So it only sets the flag on temporary files?

It's the only place where I could quickly identify it with a grep. Even for Python 3.4 this is still an open issue it seems, like this bug report shows: http://bugs.python.org/issue12107 It also discusses pros and cons a bit.

> > I think they just rely on the C library at the moment for their
> > System.IO,
> > which in turn doesn't set FD_CLOEXEC.
> 
> > Rust:
> >
> > Uses C stdlib.
> 
> Is there any discussion on why libc doesn't do it, and what do APIs that wrap the C API do?

I haven't researched any of that so far. All I know is that glibc added a new flag with 2.7 for that:

> e (since glibc 2.7)
>     Open the file with the O_CLOEXEC flag.  See open(2) for more
>     information.  This flag is ignored for fdopen().

> > Despite the increased maintenance cost I think we should adapt that behavoir in D as well.
> 
> I don't think there would be a maintenance cost to speak of. Wouldn't it be a one-line addition to a few places?

More or less. You have to query the flags, add FD_CLOEXEC and set them again. Error handling wouldn't be bad either. Its enough to write a helper function for that.

-- 
Marco