Just checking - is there any other changes that I should make to the patch, or 
anything else you guys need me to do?

Thanks,

-Andrew

-----Original Message-----
From: net-dev <net-dev-boun...@openjdk.java.net> On Behalf Of Andrew Luo
Sent: Monday, July 16, 2018 11:59 PM
To: Alan Bateman <alan.bate...@oracle.com>
Cc: net-dev@openjdk.java.net
Subject: RE: [PATCH] SOCK_CLOEXEC for opening sockets

Thanks for the feedback.  Made those changes (reverted 
src/java.base/linux/native/libnio/fs/LinuxWatchService.c):

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/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);

Thanks,

-Andrew

-----Original Message-----
From: Alan Bateman <alan.bate...@oracle.com>
Sent: Monday, July 16, 2018 11:41 PM
To: Andrew Luo <andrewluotechnolog...@outlook.com>
Cc: net-dev@openjdk.java.net
Subject: Re: [PATCH] SOCK_CLOEXEC for opening sockets

On 17/07/2018 07:33, Andrew Luo wrote:
> 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,
It's okay for the code in libnio/ch to use the NET_* functions. The issue is 
the changes to libnio/fs which is the file system code as that should have no 
dependences on functions exported by libnet. I think it would be better to drop 
the changes to the fs code in this patch. As we've been discussing, there are 
dozens of other areas that will work to allow fork/exec work reliably in native 
code, I think the networking and channels area is just the first step.

-Alan

Reply via email to