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.
