Question about libjava/childproc.c

Thomas Stüfe thomas.stuefe at gmail.com
Wed Sep 5 16:23:41 UTC 2018


On Wed, Sep 5, 2018 at 6:16 PM, Martin Buchholz <martinrb at google.com> wrote:
> Alan: Thomas seems to be suggesting setting the FD_CLOEXEC flag after fork
> but before exec, which is a slightly different idea.
>
> Thomas: This is an interesting idea.  Historically the usual strategy was to
> close all the file descriptors explicitly, perhaps before FD_CLOEXEC was
> something we could rely on.  One might expect the explicit close to be more
> efficient, since it involves fewer calls into libc.  As always, the process
> code is rather brittle and we don't like to touch it if it's working.
>
> We would probably want to use the idiom at
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
>
>     flags = fcntl(fd, F_GETFD);
>     if (flags == -1)
>         /* Handle error */;
>     flags |= FD_CLOEXEC;
>     if (fcntl(fd, F_SETFD, flags) == -1)
>         /* Handle error */;"
>

Of course. My example removed all other flags. Thanks for pointing that out :)

However, I believe we do not really know if the current coding always
works, or? If we accidentally close the opendir file descriptor,
because the magic workaround above did not work, readdir() would
probably just fail and we would silently exit the loop, leaving the
remaining file descriptors open?

Since all this happens between vfork and exec we cannot do any decent
error handling here. Even what little we do is way outside the allowed
spec for vfork().

..Thomas

>
>
>
> On Wed, Sep 5, 2018 at 9:06 AM, Alan Bateman <Alan.Bateman at oracle.com>
> wrote:
>>
>> On 05/09/2018 16:45, Thomas Stüfe wrote:
>>>
>>> :
>>>
>>> My question would be, could we not - instead of straight away closing
>>> the file descriptor - set them all to FD_CLOEXEC instead?
>>>
>> This comes up periodically but even if we do that then we still need this
>> code to catch the places where FD_CLOEXEC isn't set.
>>
>> Note that there a thread net-dev trying to do this for sockets. The
>> scenario there seems to be someone calling fork/exec directory and not using
>> ProcessBuilder. The patch under discussion is not complete but it helps.
>>
>> -Alan
>
>


More information about the core-libs-dev mailing list