Sorry, I meant to say: "Unfortunately, that is only for files... sockets in Linux are file descriptors as well, but OpenJDK is (currently) not opening them with CLOEXEC." -----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On Behalf Of Andrew Luo Sent: Wednesday, October 24, 2018 1:18 PM To: Thomas Stüfe <thomas.stuefe@gmail.com>; Martin Buchholz <martinrb@google.com> Cc: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: RE: RFR(s): 8210549: Runtime.exec: in closeDescriptors(), use FD_CLOEXEC instead of close() By the way, there is some related work on CLOEXEC for networking that I had done. It seems like this work is somewhat similar (although my work addresses a different use case - when JNI code calls exec on its own and doesn't go through and close its inherited FDs): http://mail.openjdk.java.net/pipermail/net-dev/2018-August/011730.html "> If I understand this correctly, this only affects stray JVM/JDK file
descriptors which were not opened with FD_CLOEXEC - which is an error - and third party native code. In hotspot, os::open() sets FD_CLOEXEC, and java file descriptors are set to FD_CLOEXEC too."
Unfortunately, that is only for sockets... sockets in Linux are file descriptors as well, but OpenJDK is (currently) not opening them with CLOEXEC. Thanks, -Andrew -----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On Behalf Of Thomas Stüfe Sent: Wednesday, October 24, 2018 11:18 AM To: Martin Buchholz <martinrb@google.com> Cc: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: Re: RFR(s): 8210549: Runtime.exec: in closeDescriptors(), use FD_CLOEXEC instead of close() "Do we need the fallback on your beloved HP-UX?" We do not ship OpenJDK on HPUX, so I do not really care. And I really do not love it :) ..Thomas On Wed, Oct 24, 2018 at 8:16 PM Thomas Stüfe <thomas.stuefe@gmail.com> wrote:
Hi Martin,
sorry for letting this rest for so long - I got sidetracked by other issues.
On Wed, Sep 12, 2018 at 3:17 PM Martin Buchholz <martinrb@google.com> wrote:
In 2018, we can assume everyone has implemented FD_CLOEXEC.
Back in 1970, 20 file descriptors was enough for any program, but today they're effectively unlimited, so futilely closing _SC_OPEN_MAX descriptors has become unreasonable (as you say). OTOH, there is still no standard way of ensuring all our file descriptors are closed. Do we need the fallback on your beloved HP-UX? Should we simply close the first 64k file descriptors and pray, as is traditional?
This sounds like a reasonable limit.
If I understand this correctly, this only affects stray JVM/JDK file descriptors which were not opened with FD_CLOEXEC - which is an error - and third party native code. In hotspot, os::open() sets FD_CLOEXEC, and java file descriptors are set to FD_CLOEXEC too.
So the error surface is quiet small nowadays. So I think giving up after 64K is okay.
freebsd offers closefrom(2) https://www.freebsd.org/cgi/man.cgi?query=closefrom&sektion=2&manpat h=freebsd-release-ports and today we can autoconfiscate the existence of closefrom and use that where available (BSD and Solaris?)
Here is what I think is theoretically possible:
AIX: has a fcntl command which closes all file descriptors above X which is fine
BSD (but apparently not MacOS): has int closefrom(2) which is fine
Solaris: has a useless closefrom(3) which returns no exit code. Unless one wants to ignore the exit code, this is no option. However, Solaris has fdwalk(3), which we use in our port to walk open file descriptors and close them. But I am not sure about the merit here, this may be actually more expensive than walking the proc file system itself.
-----
In any case I would suggest to keep the fallback code, which walks the proc file system, for all Unices. And also to keep the fallback from that, which closes a range brute force - possibly with a reasonable max. Is that too strict?
The question is now what is more important to you: code clarity (few platform ifdefs) or trying out every technique we can think of?
A) If code clarity and simplicity is important, I would basically keep the coding we have now: just walk the proc file system - including the AIX ifdefs unfortunately - and as fallback close the range with brute force until reasonable max.
B) If we want to try everything, we will have more ifdefs, not less, which would look something like this:
#ifdef AIX try fcntl(F_CLOSEM) #endif
#if defined(_ALL_BSD_SOURCE) && !defined(APPLE) try closefrom() #endif
#ifdef SOLARIS try fdwalk (?) #endif
fallback: walk proc file system
fallback: close range until 64K or so.
------
Not sure this is worth it...
In our port I did (B) but today I personally would go with (A). (B) has too many difficult-to-test-branches. And it may be actually slower, since you may do work twice (if e.g. fdwalk fails for the same reason that manually walking the proc fs fails).
What do you think?
closeDescriptors needs a better name now - maybe ensureDescriptorsAreClosed ?
I'd give this function a javadoc style docstring.
Will fix both.
(Finding the FD_DIR directory has become rather ugly, especially the AIX-specific code)
Sorry.
I keep hoping that sometime in the future IBM will actually take part in this AIX port and share some of the work. And blame :(
Best Regards, Thomas