8194734 : Handle to jimage file inherited into child processes (win)
martinrb at google.com
Fri Apr 6 21:01:26 UTC 2018
The design on Unix was that all file descriptors greater than 3 are closed
on fork and before exec.
But regardless of that, we should try to set the CLOEXEC bit on all our
file descriptor (who knows, there might be native code that does fork+exec).
File descriptor 3 is indeed deep magic, used for error reporting. It's a
serious bug if FAIL_FILENO ends up being used for something other than the
error reporting pipe.
I don't think I'm the last (or the first!) person who touched this.
On Fri, Apr 6, 2018 at 12:39 PM, Alexander Miloslavskiy <
alexandr.miloslavskiy at gmail.com> wrote:
> > The update to osSupport_Windows.cpp looks okay. I think it would be
> > useful to discuss the osSupport_unix.cpp change further as the
> > childproc code is supported to close the file descriptors.
> childproc code uses some dark magic to figure which files to close. At the
> moment, closeDescriptors() will close all files which has descriptors
> bigger then from_fd (4). However, jimage ends up having descriptor 3, so
> it's not closed. On the other hand, childproc expects descriptor 3 to be
> occupied by a pipe to parent (FAIL_FILENO). This means that currently, dark
> magic is already broken. The only reason for jimage to not go into child
> process is being mistaken with FAIL_FILENO.
> One way of fixing bug in childproc could be increasing FAIL_FILENO to
> match its actual value of file descriptor. This way, jimage will no longer
> be mistaken with FAIL_FILENO and therefore, it will NOT be closed.
> With all this in mind, I believe it wouldn't hurt to explicitly say that
> jimage must not be inherited, without relying on current implementation of
> childproc. That makes a sturdy and clean code.
> When this patch is accepted, I can also work on fixing bugs in childproc,
> if that is considered worthy.
More information about the core-libs-dev