bug#14803: Setting close-on-exec flag consistently

[Eli Zaretskii]

> Date: Sat, 06 Jul 2013 10:18:07 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 14803@debbugs.gnu.org
> 
> On 07/06/2013 04:31 AM, Eli Zaretskii wrote:
> > The Windows build has its own implementations
> > of 'pipe' and 'socket' (because it needs to make them behave like
> > Posix APIs, and also because it needs to watch each subprocess and
> > each socket for the benefit of 'select', 'waitpid', SIGCHLD, etc.).
> > So the replacements from gnulib won't do.
> 
> The gnulib replacements do that sort of thing too.  Emacs
> uses a hybrid approach, which uses gnulib for some things
> and its own (pre-gnulib) solution for others, and it should
> be possible to keep the hybrid working in this case.  In the
> long run it may be better to merge the two approaches rather
> than maintain a hybrid

I don't know how easy that would be, since the infrastructure used in
Emacs for this is very elaborate, and might contradict with
general-purpose use of gnulib.

> but that issue doesn't need to be addressed now.

Right.

> > Socket handles _are_ inherited, but making them non-inheritable is not
> > easy (some say impossible in the general case), and since no one
> > complained till now, I'm willing to live with that until we hear some
> > real problems.
> 
> That's the intent of this patch.  On systems like w32 that
> lack SOCK_CLOEXEC, the code creates sockets just as it did
> before (using plain 'socket' and 'accept' rather than
> 'socket' and 'accept4' with SOCK_CLOEXEC).

Given that w32 redirects calls to 'socket' to 'sys_socket', which is
implemented on w32.c, will the above still work?

> The only difference is that the proposed code then invokes fcntl
> (fd, F_SETFD, FD_CLOEXEC) to set the close-on-exec flag; this is
> needed on POSIXish systems that don't have SOCK_CLOEXEC.  From what
> you say, on w32 platforms it's OK if this is a no-op.  w32.c's fcntl
> currently returns SOCKET_ERROR for this fcntl call, which should be
> fine, since Emacs doesn't check for an error.
> 
> > For mktemp/mkstemp in filelock.c, sys_open in w32.c already makes all
> > handles not inheritable by default, so that problem doesn't exist,
> > either.
> 
> This should be OK, since the w32 port doesn't define
> HAVE_MKOSTEMP, so Emacs should use mkstemp or mktemp as it
> did before.  Emacs will now invoke fcntl (fd, F_SETFD,
> FD_CLOEXEC) on the resulting file descriptor but that should
> be OK as described above.
> 
> > As for emacs_open, it needs to use O_NOINHERIT instead of O_CLOEXEC
> > (or just use zero, since w32.c already adds the O_NOINHERIT flag in
> > sys_open).
> 
> The proposed patch does this, as fcntl.h should #define
> O_CLOEXEC to O_NOINHERIT on w32.
> 
> > So I think we should simply not compile lib/fcntl.c and lib/pipe2.c
> > (including their dependency modules) on MS-Windows.
> 
> That should be OK.  w32.c will need minor tweaks to support
> F_DUPFD_CLOEXEC and pipe2, something along the following lines.

I'm OK with the rest of your proposals.  I assume that lib/fcntl.c and
lib/pipe2.c will not be compiled for w32, due to some configure test?
Or did you have something else in mind?

Thanks.