I agree with this approach - it parallels the efforts made with O_CLOEXEC in past years.
According to https://www.freebsd.org/cgi/man.cgi?query=socket&sektion=2 SOCK_CLOEXEC is also available on freebsd. On Tue, Jul 10, 2018 at 1:36 AM, Andrew Luo < andrewluotechnolog...@outlook.com> wrote: > Hi, > > > > I want to propose to use SOCK_CLOEXEC when opening sockets in the > OpenJDK. I ran into some issues when forking processes (in > JNI/C/C++/native code) on Linux where OpenJDK had opened a socket (in Java > code). The child process ends up inheriting a file descriptor to the same > socket, which is not ideal in certain circumstances (for example if the > Java process restarts and tries to open the socket again while the child > process is still running). Of course, after forking the child process can > close all those file descriptors as a workaround, but we use O_CLOEXEC when > opening files, so I think it would be ideal to use the same for sockets. > > > > Just some info about the patch: > > > > 1. This is only for Linux (I don’t believe SOCK_CLOEXEC exists on > other platforms, I use a preprocessor guard for SOCK_CLOEXEC) > > 2. I try twice if the first time attempting to open the socket > fails with EINVAL because it is possible that the OpenJDK was compiled on a > newer kernel/with newer headers that supports SOCK_CLOEXEC but runs on a > lower version kernel (not sure if this is supported by the OpenJDK project) > > > > Patch is attached below. Let me know if you want me to make some changes. > > > > (I did not add a unit test – it would probably need to be a functional > test, one that involves a child process and forking, etc. Let me know if > you believe this is necessary to add) > > > > Thanks, > > > > -Andrew > > > > diff -r 95c0644a1c47 src/java.base/unix/native/libnet/PlainSocketImpl.c > > --- a/src/java.base/unix/native/libnet/PlainSocketImpl.c Fri Jun 15 > 17:34:01 2018 -0700 > > +++ b/src/java.base/unix/native/libnet/PlainSocketImpl.c Tue > Jul 10 01:30:08 2018 -0700 > > @@ -104,6 +104,34 @@ > > } > > /* > > + * Opens a socket > > + * On systems where supported, uses SOCK_CLOEXEC where possible > > + */ > > +static int openSocket(int domain, int type, int protocol) { > > +#if defined(SOCK_CLOEXEC) > > + int typeToUse = type | SOCK_CLOEXEC; > > +#else > > + int typeToUse = type; > > +#endif > > + > > + int socketFileDescriptor = socket(domain, typeToUse, protocol); > > +#if defined(SOCK_CLOEXEC) > > + if ((socketFileDescriptor == -1) && (errno = EINVAL)) { > > + // Attempt to open the socket without SOCK_CLOEXEC > > + // May have been compiled on an OS with SOCK_CLOEXEC supported > > + // But runtime system might not have SOCK_CLOEXEC support > > + socketFileDescriptor = socket(domain, type, protocol); > > + } > > +#else > > + // Best effort > > + // Return value is intentionally ignored since socket was > successfully opened anyways > > + fcntl(socketFileDescriptor, F_SETFD, FD_CLOEXEC); > > +#endif > > + > > + return socketFileDescriptor; > > +} > > + > > +/* > > * The initroto function is called whenever PlainSocketImpl is > > * loaded, to cache field IDs for efficiency. This is called every time > > * the Java class is loaded. > > @@ -178,7 +206,8 @@ > > return; > > } > > - if ((fd = socket(domain, type, 0)) == -1) { > > + > > + if ((fd = openSocket(domain, type, 0)) == -1) { > > /* note: if you run out of fds, you may not be able to load > > * the exception class, and get a NoClassDefFoundError > > * instead. >
