Great, thanks. By the way, I do see other places where we use NET_* functions in libnio, but if you prefer that I duplicate that code instead, I can do that.
By the way, I do see other places where libnio calls into the NET_* functions: jdk/src/java.base/unix/native/libnio$ grep -F -r --include='*.c' 'NET_' . ./ch/DatagramChannelImpl.c: if (!NET_SockaddrEqualsInetAddress(env, &sa, senderAddr)) { ./ch/DatagramChannelImpl.c: if (port != NET_GetPortFromSockaddr(&sa)) { ./ch/DatagramChannelImpl.c: jobject ia = NET_SockaddrToInetAddress(env, &sa, &port); ./ch/DatagramChannelImpl.c: NET_GetPortFromSockaddr(&sa)); ./ch/DatagramChannelImpl.c: if (NET_InetAddressToSockaddr(env, destAddress, destPort, &sa, ./ch/Net.c: fd = NET_Socket(domain, type, 0); ./ch/Net.c: if (NET_InetAddressToSockaddr(env, iao, port, &sa, &sa_len, ./ch/Net.c: rv = NET_Bind(fdval(env, fdo), &sa, sa_len); ./ch/Net.c: if (NET_InetAddressToSockaddr(env, iao, port, &sa, &sa_len, preferIPv6) != 0) { ./ch/Net.c: return NET_GetPortFromSockaddr(&sa); ./ch/Net.c: return NET_SockaddrToInetAddress(env, &sa, &port); ./ch/Net.c: n = NET_GetSockOpt(fdval(env, fdo), level, opt, arg, (int*)&arglen); ./ch/Net.c: n = NET_SetSockOpt(fdval(env, fdo), level, opt, parg, arglen); ./ch/ServerSocketChannelImpl.c: remote_ia = NET_SockaddrToInetAddress(env, &sa, (int *)&remote_port); ./ch/InheritedChannel.c: /* Initialize InetAddress IDs before later use of NET_XXX functions */ ./ch/InheritedChannel.c: remote_ia = NET_SockaddrToInetAddress(env, &sa, (int *)&remote_port); ./ch/InheritedChannel.c: NET_SockaddrToInetAddress(env, &sa, (int *)&remote_port); ./ch/FileDispatcherImpl.c: if (NET_SocketPair(PF_UNIX, SOCK_STREAM, 0, sp) < 0) { Here's my updated patch: diff --git a/make/lib/Lib-jdk.net.gmk b/make/lib/Lib-jdk.net.gmk --- a/make/lib/Lib-jdk.net.gmk +++ b/make/lib/Lib-jdk.net.gmk @@ -35,7 +35,7 @@ CFLAGS := $(CFLAGS_JDKLIB), \ LDFLAGS := $(LDFLAGS_JDKLIB) \ $(call SET_SHARED_LIBRARY_ORIGIN), \ - LIBS := -ljava, \ + LIBS := -ljava -lnet, \ LIBS_solaris := -lsocket, \ LIBS_linux := -ljvm, \ )) diff --git a/src/java.base/aix/native/libnio/ch/AixPollPort.c b/src/java.base/aix/native/libnio/ch/AixPollPort.c --- a/src/java.base/aix/native/libnio/ch/AixPollPort.c +++ b/src/java.base/aix/native/libnio/ch/AixPollPort.c @@ -137,7 +137,7 @@ JNIEXPORT void JNICALL Java_sun_nio_ch_AixPollPort_socketpair(JNIEnv* env, jclass clazz, jintArray sv) { int sp[2]; - if (socketpair(PF_UNIX, SOCK_STREAM, 0, sp) == -1) { + if (NET_SocketPair(PF_UNIX, SOCK_STREAM, 0, sp) == -1) { JNU_ThrowIOExceptionWithLastError(env, "socketpair failed"); } else { jint res[2]; diff --git a/src/java.base/linux/native/libnio/fs/LinuxWatchService.c b/src/java.base/linux/native/libnio/fs/LinuxWatchService.c --- a/src/java.base/linux/native/libnio/fs/LinuxWatchService.c +++ b/src/java.base/linux/native/libnio/fs/LinuxWatchService.c @@ -119,7 +119,7 @@ (JNIEnv* env, jclass clazz, jintArray sv) { int sp[2]; - if (socketpair(PF_UNIX, SOCK_STREAM, 0, sp) == -1) { + if (NET_SocketPair(PF_UNIX, SOCK_STREAM, 0, sp) == -1) { throwUnixException(env, errno); } else { jint res[2]; diff --git a/src/java.base/unix/native/libnet/Inet4AddressImpl.c b/src/java.base/unix/native/libnet/Inet4AddressImpl.c --- a/src/java.base/unix/native/libnet/Inet4AddressImpl.c +++ b/src/java.base/unix/native/libnet/Inet4AddressImpl.c @@ -264,7 +264,7 @@ int connect_rv = -1; // open a TCP socket - fd = socket(AF_INET, SOCK_STREAM, 0); + fd = NET_Socket(AF_INET, SOCK_STREAM, 0); if (fd == -1) { // note: if you run out of fds, you may not be able to load // the exception class, and get a NoClassDefFoundError instead. @@ -503,7 +503,7 @@ // Let's try to create a RAW socket to send ICMP packets. // This usually requires "root" privileges, so it's likely to fail. - fd = socket(AF_INET, SOCK_RAW, IPPROTO_ICMP); + fd = NET_Socket(AF_INET, SOCK_RAW, IPPROTO_ICMP); if (fd == -1) { return tcp_ping4(env, &sa, netif, timeout, ttl); } else { diff --git a/src/java.base/unix/native/libnet/Inet6AddressImpl.c b/src/java.base/unix/native/libnet/Inet6AddressImpl.c --- a/src/java.base/unix/native/libnet/Inet6AddressImpl.c +++ b/src/java.base/unix/native/libnet/Inet6AddressImpl.c @@ -461,7 +461,7 @@ int connect_rv = -1; // open a TCP socket - fd = socket(AF_INET6, SOCK_STREAM, 0); + fd = NET_Socket(AF_INET6, SOCK_STREAM, 0); if (fd == -1) { // note: if you run out of fds, you may not be able to load // the exception class, and get a NoClassDefFoundError instead. @@ -711,7 +711,7 @@ // Let's try to create a RAW socket to send ICMP packets. // This usually requires "root" privileges, so it's likely to fail. - fd = socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6); + fd = NET_Socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6); if (fd == -1) { return tcp_ping6(env, &sa, netif, timeout, ttl); } else { diff --git a/src/java.base/unix/native/libnet/NetworkInterface.c b/src/java.base/unix/native/libnet/NetworkInterface.c --- a/src/java.base/unix/native/libnet/NetworkInterface.c +++ b/src/java.base/unix/native/libnet/NetworkInterface.c @@ -1055,7 +1055,7 @@ static int openSocket(JNIEnv *env, int proto) { int sock; - if ((sock = socket(proto, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(proto, SOCK_DGRAM, 0)) < 0) { // If EPROTONOSUPPORT is returned it means we don't have // support for this proto so don't throw an exception. if (errno != EPROTONOSUPPORT) { @@ -1078,9 +1078,9 @@ static int openSocketWithFallback(JNIEnv *env, const char *ifname) { int sock; - if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET, SOCK_DGRAM, 0)) < 0) { if (errno == EPROTONOSUPPORT) { - if ((sock = socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { JNU_ThrowByNameWithMessageAndLastError (env, JNU_JAVANETPKG "SocketException", "IPV6 Socket creation failed"); return -1; @@ -1315,9 +1315,9 @@ static int openSocketWithFallback(JNIEnv *env, const char *ifname) { int sock; - if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET, SOCK_DGRAM, 0)) < 0) { if (errno == EPROTONOSUPPORT) { - if ((sock = socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { JNU_ThrowByNameWithMessageAndLastError (env, JNU_JAVANETPKG "SocketException", "IPV6 Socket creation failed"); return -1; @@ -1590,9 +1590,9 @@ int sock, alreadyV6 = 0; struct lifreq if2; - if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET, SOCK_DGRAM, 0)) < 0) { if (errno == EPROTONOSUPPORT) { - if ((sock = socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { JNU_ThrowByNameWithMessageAndLastError (env, JNU_JAVANETPKG "SocketException", "IPV6 Socket creation failed"); return -1; @@ -1616,7 +1616,7 @@ strncpy(if2.lifr_name, ifname, sizeof(if2.lifr_name) - 1); if (ioctl(sock, SIOCGLIFNETMASK, (char *)&if2) < 0) { close(sock); - if ((sock = socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { JNU_ThrowByNameWithMessageAndLastError (env, JNU_JAVANETPKG "SocketException", "IPV6 Socket creation failed"); return -1; @@ -1941,9 +1941,9 @@ static int openSocketWithFallback(JNIEnv *env, const char *ifname) { int sock; - if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET, SOCK_DGRAM, 0)) < 0) { if (errno == EPROTONOSUPPORT) { - if ((sock = socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { + if ((sock = NET_Socket(AF_INET6, SOCK_DGRAM, 0)) < 0) { JNU_ThrowByNameWithMessageAndLastError (env, JNU_JAVANETPKG "SocketException", "IPV6 Socket creation failed"); return -1; diff --git a/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c b/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c --- a/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c +++ b/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c @@ -904,7 +904,7 @@ return; } - if ((fd = socket(domain, SOCK_DGRAM, 0)) == -1) { + if ((fd = NET_Socket(domain, SOCK_DGRAM, 0)) == -1) { JNU_ThrowByNameWithMessageAndLastError (env, JNU_JAVANETPKG "SocketException", "Error creating socket"); return; diff --git a/src/java.base/unix/native/libnet/PlainSocketImpl.c b/src/java.base/unix/native/libnet/PlainSocketImpl.c --- a/src/java.base/unix/native/libnet/PlainSocketImpl.c +++ b/src/java.base/unix/native/libnet/PlainSocketImpl.c @@ -77,7 +77,7 @@ int sv[2]; #ifdef AF_UNIX - if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) == -1) { + if (NET_SocketPair(AF_UNIX, SOCK_STREAM, 0, sv) == -1) { return -1; } #else @@ -178,7 +178,7 @@ return; } - if ((fd = socket(domain, type, 0)) == -1) { + if ((fd = NET_Socket(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. diff --git a/src/java.base/unix/native/libnet/SdpSupport.c b/src/java.base/unix/native/libnet/SdpSupport.c --- a/src/java.base/unix/native/libnet/SdpSupport.c +++ b/src/java.base/unix/native/libnet/SdpSupport.c @@ -57,7 +57,7 @@ #if defined(__solaris__) int domain = ipv6_available() ? AF_INET6 : AF_INET; - s = socket(domain, SOCK_STREAM, PROTO_SDP); + s = NET_Socket(domain, SOCK_STREAM, PROTO_SDP); #elif defined(__linux__) /** * IPv6 not supported by SDP on Linux @@ -66,7 +66,7 @@ JNU_ThrowIOException(env, "IPv6 not supported"); return -1; } - s = socket(AF_INET_SDP, SOCK_STREAM, 0); + s = NET_Socket(AF_INET_SDP, SOCK_STREAM, 0); #else /* not supported on other platforms at this time */ s = -1; diff --git a/src/java.base/unix/native/libnet/net_util_md.c b/src/java.base/unix/native/libnet/net_util_md.c --- a/src/java.base/unix/native/libnet/net_util_md.c +++ b/src/java.base/unix/native/libnet/net_util_md.c @@ -117,6 +117,64 @@ return defaultIndex; } +/* + * Opens a socket + * On systems where supported, uses SOCK_CLOEXEC where possible + */ +JNIEXPORT int JNICALL +NET_Socket(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 + if (socketFileDescriptor != -1) { + // Best effort + // Return value is intentionally ignored since socket was successfully opened anyways + fcntl(socketFileDescriptor, F_SETFD, FD_CLOEXEC); + } +#endif + + return socketFileDescriptor; +} + +JNIEXPORT int JNICALL +NET_SocketPair(int domain, int type, int protocol, int socket_vector[2]) { +#if defined(SOCK_CLOEXEC) + int typeToUse = type | SOCK_CLOEXEC; +#else + int typeToUse = type; +#endif + + int socketFileDescriptor = socketpair(domain, typeToUse, protocol, socket_vector); +#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 = socketpair(domain, type, protocol, socket_vector); + } +#else + if (socketFileDescriptor != -1) { + // Best effort + // Return value is intentionally ignored since socket was successfully opened anyways + fcntl(socketFileDescriptor, F_SETFD, FD_CLOEXEC); + } +#endif + + return socketFileDescriptor; +} + #define RESTARTABLE(_cmd, _result) do { \ do { \ _result = _cmd; \ @@ -295,7 +353,7 @@ SOCKETADDRESS sa; socklen_t sa_len = sizeof(SOCKETADDRESS); - fd = socket(AF_INET6, SOCK_STREAM, 0) ; + fd = NET_Socket(AF_INET6, SOCK_STREAM, 0) ; if (fd < 0) { /* * TODO: We really cant tell since it may be an unrelated error @@ -402,7 +460,7 @@ /* Do a simple dummy call, and try to figure out from that */ int one = 1; int rv, s; - s = socket(PF_INET, SOCK_STREAM, 0); + s = NET_Socket(PF_INET, SOCK_STREAM, 0); if (s < 0) { return JNI_FALSE; } diff --git a/src/java.base/unix/native/libnet/net_util_md.h b/src/java.base/unix/native/libnet/net_util_md.h --- a/src/java.base/unix/native/libnet/net_util_md.h +++ b/src/java.base/unix/native/libnet/net_util_md.h @@ -89,6 +89,8 @@ int NET_Writev(int s, const struct iovec * vector, int count); int NET_Connect(int s, struct sockaddr *addr, int addrlen); int NET_Accept(int s, struct sockaddr *addr, socklen_t *addrlen); +JNIEXPORT int JNICALL NET_Socket(int domain, int type, int protocol); +JNIEXPORT int JNICALL NET_SocketPair(int domain, int type, int protocol, int socket_vector[2]); int NET_SocketClose(int s); int NET_Dup2(int oldfd, int newfd); int NET_Poll(struct pollfd *ufds, unsigned int nfds, int timeout); diff --git a/src/java.base/unix/native/libnio/ch/FileDispatcherImpl.c b/src/java.base/unix/native/libnio/ch/FileDispatcherImpl.c --- a/src/java.base/unix/native/libnio/ch/FileDispatcherImpl.c +++ b/src/java.base/unix/native/libnio/ch/FileDispatcherImpl.c @@ -67,7 +67,7 @@ Java_sun_nio_ch_FileDispatcherImpl_init(JNIEnv *env, jclass cl) { int sp[2]; - if (socketpair(PF_UNIX, SOCK_STREAM, 0, sp) < 0) { + if (NET_SocketPair(PF_UNIX, SOCK_STREAM, 0, sp) < 0) { JNU_ThrowIOExceptionWithLastError(env, "socketpair failed"); return; } diff --git a/src/java.base/unix/native/libnio/ch/Net.c b/src/java.base/unix/native/libnio/ch/Net.c --- a/src/java.base/unix/native/libnio/ch/Net.c +++ b/src/java.base/unix/native/libnio/ch/Net.c @@ -198,7 +198,7 @@ int type = (stream ? SOCK_STREAM : SOCK_DGRAM); int domain = (ipv6_available() && preferIPv6) ? AF_INET6 : AF_INET; - fd = socket(domain, type, 0); + fd = NET_Socket(domain, type, 0); if (fd < 0) { return handleSocketError(env, errno); } diff --git a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c --- a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c +++ b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c @@ -86,7 +86,7 @@ (JNIEnv *env, jobject unused) { int one = 1; int rv, s; - s = socket(PF_INET, SOCK_STREAM, 0); + s = NET_Socket(PF_INET, SOCK_STREAM, 0); if (s < 0) { return JNI_FALSE; } @@ -103,7 +103,7 @@ static jint socketOptionSupported(jint sockopt) { jint one = 1; jint rv, s; - s = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); + s = NET_Socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); if (s < 0) { return 0; } diff --git a/src/jdk.net/macosx/native/libextnet/MacOSXSocketOptions.c b/src/jdk.net/macosx/native/libextnet/MacOSXSocketOptions.c --- a/src/jdk.net/macosx/native/libextnet/MacOSXSocketOptions.c +++ b/src/jdk.net/macosx/native/libextnet/MacOSXSocketOptions.c @@ -35,7 +35,7 @@ static jint socketOptionSupported(jint sockopt) { jint one = 1; jint rv, s; - s = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); + s = NET_Socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); if (s < 0) { return 0; } diff --git a/src/jdk.net/solaris/native/libextnet/SolarisSocketOptions.c b/src/jdk.net/solaris/native/libextnet/SolarisSocketOptions.c --- a/src/jdk.net/solaris/native/libextnet/SolarisSocketOptions.c +++ b/src/jdk.net/solaris/native/libextnet/SolarisSocketOptions.c @@ -157,7 +157,7 @@ sock_flow_props_t props; int rv, s; - s = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); + s = NET_Socket(PF_INET, SOCK_STREAM, IPPROTO_TCP); if (s < 0) { return JNI_FALSE; } diff --git a/src/jdk.sctp/unix/native/libsctp/SctpNet.c b/src/jdk.sctp/unix/native/libsctp/SctpNet.c --- a/src/jdk.sctp/unix/native/libsctp/SctpNet.c +++ b/src/jdk.sctp/unix/native/libsctp/SctpNet.c @@ -151,7 +151,7 @@ Java_sun_nio_ch_sctp_SctpNet_init (JNIEnv *env, jclass cl) { int sp[2]; - if (socketpair(PF_UNIX, SOCK_STREAM, 0, sp) < 0) { + if (NET_SocketPair(PF_UNIX, SOCK_STREAM, 0, sp) < 0) { JNU_ThrowIOExceptionWithLastError(env, "socketpair failed"); return; } @@ -180,7 +180,7 @@ return 0; } - fd = socket(domain, (oneToOne ? SOCK_STREAM : SOCK_SEQPACKET), IPPROTO_SCTP); + fd = NET_Socket(domain, (oneToOne ? SOCK_STREAM : SOCK_SEQPACKET), IPPROTO_SCTP); if (fd < 0) { return handleSocketError(env, errno); By the way, I only have x86 systems (and VMs) - any way I can test this on AIX, or what is the preferred way to ensure my changes will build properly on all platforms? Thanks, -Andrew -----Original Message----- From: Chris Hegarty <chris.hega...@oracle.com> Sent: Monday, July 16, 2018 2:58 AM To: Andrew Luo <andrewluotechnolog...@outlook.com> Cc: net-dev@openjdk.java.net Subject: Re: [PATCH] SOCK_CLOEXEC for opening sockets Andrew, I filed the following issue to track this: https://bugs.openjdk.java.net/browse/JDK-8207335 Once you produce an updated patch, I can review and sponsor this for you. -Chris. On 12/07/18 08:21, Andrew Luo wrote: > Thanks, I can refactor it. I'm not as familiar with the OpenJDK > architecture - should I just duplicate the function into libnio or is > there some common utility library that I should move it into? Also, > let me know what in net_util_* needs cleanup. The other instances of > socket/socketpair in Mac/AIX/Solaris I'm willing to handle as well. > > Files are mostly already handled: > https://bugs.openjdk.java.net/browse/JDK-8043780 (in a similar way - > O_CLOEXEC on systems that support it and fcntl on other POSIX systems). > One other resource you mentioned is epolls. Perhaps there's also > other file descriptors that are being used elsewhere, but I think it > will take me longer time to figure out. I'm willing to work on that > as well if we all think it's a good idea. > > Anyways, while I agree that it would be ideal to address those other > file descriptors as well, is it ok to address those in a separate > changeset, or do you think that this changeset doesn't really provide > value unless we make this change everywhere? Sockets are somewhat > more problematic than other types of file descriptors (in my opinion) > as if you use it to listen on a port and then fork child processes > (from native code) then restart the parent process, it won't be able > to listen to the same port until all of the children processes (of the > previous > process) exit/close the fd. Other file descriptors, unless you open > in exclusive mode, won't have the same problem (although the behavior > is undesirable). > > Thanks, > > -Andrew > > *From:*Alan Bateman <alan.bate...@oracle.com> > *Sent:* Wednesday, July 11, 2018 11:59 PM > *To:* Andrew Luo <andrewluotechnolog...@outlook.com> > *Cc:* Martin Buchholz <marti...@google.com>; net-dev@openjdk.java.net > *Subject:* Re: [PATCH] SOCK_CLOEXEC for opening sockets > > On 12/07/2018 05:55, Andrew Luo wrote: > > Ok, fixed a few more places (and a bug where fcntl was being run on > a -1 fd). Patch is below, let me know if there's any other > suggestions/etc. > > The file system code should not be calling into NET_* functions. The > changes to net_util_* will also need cleanup. Otherwise I think you've > got all the places in the networking / NIO APIs, at least on Linux. > There are others on macOS, Solaris, ... of course. > > That said, I assume we have many more places in the JDK that have > file descriptors to open files and other resources. If we really want > to support the case of someone calling fork/exec from JNI code then it > will likely need work in several other areas. > > -Alan >