Re: [PATCH] SOCK_CLOEXEC for opening sockets
On 12/07/2018 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. JDK-8043780 was in libjvm so it doesn't cover file and networking I/O or other file descriptors created in the libraries. I agree with your comment that this could be done in steps as there are many areas of the libraries that would changes to work with code that uses fork/exec directly rather than ProcessBuilder/Process. It's okay to start with the socket and channel APIs (native code in libnet and libnio). -Alan.
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 *Sent:* Wednesday, July 11, 2018 11:59 PM *To:* Andrew Luo *Cc:* Martin Buchholz ; 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
Re: RFR [11] 8207265: Bad HTML in {@link} in HttpResponse.BodySubscribers.ofPublisher
Looks fine Chris.
- Michael.
On 16/07/2018, 12:10, Chris Hegarty wrote:
This is a review request for a small doc-only change to fix a
"bad" use of angle brackets ( for a parameterized type ) as
the target of an @link [*]. The simplest solution is to just
replace the link with @code, since the built javadoc contains
the appropriate links in the method declaration.
diff --git
a/src/java.net.http/share/classes/java/net/http/HttpResponse.java
b/src/java.net.http/share/classes/java/net/http/HttpResponse.java
--- a/src/java.net.http/share/classes/java/net/http/HttpResponse.java
+++ b/src/java.net.http/share/classes/java/net/http/HttpResponse.java
@@ -1188,7 +1188,7 @@
/**
* Returns a response subscriber which publishes the response
body
- * through a {@link Publisher Publisher>}.
+ * through a {@code Publisher>}.
*
* The {@link HttpResponse} using this subscriber is
available
* immediately after the response headers have been read,
without
-Chris.
[*] https://bugs.openjdk.java.net/browse/JDK-8207265
RFR [11] 8207265: Bad HTML in {@link} in HttpResponse.BodySubscribers.ofPublisher
This is a review request for a small doc-only change to fix a
"bad" use of angle brackets ( for a parameterized type ) as
the target of an @link [*]. The simplest solution is to just
replace the link with @code, since the built javadoc contains
the appropriate links in the method declaration.
diff --git
a/src/java.net.http/share/classes/java/net/http/HttpResponse.java
b/src/java.net.http/share/classes/java/net/http/HttpResponse.java
--- a/src/java.net.http/share/classes/java/net/http/HttpResponse.java
+++ b/src/java.net.http/share/classes/java/net/http/HttpResponse.java
@@ -1188,7 +1188,7 @@
/**
* Returns a response subscriber which publishes the response body
- * through a {@link Publisher Publisher>}.
+ * through a {@code Publisher>}.
*
* The {@link HttpResponse} using this subscriber is available
* immediately after the response headers have been read, without
-Chris.
[*] https://bugs.openjdk.java.net/browse/JDK-8207265
