On Mon, 27 Jul 2026 07:07:10 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review of this test-only fix to the > `test/jdk/java/net/Socket/SocketReadInterruptTest.java` test that fails > intermittently? This addresses https://bugs.openjdk.org/browse/JDK-8383768. > > The test was introduced back in JDK 16 when addressing > https://bugs.openjdk.org/browse/JDK-8237858. The (old) `PlainSocketImpl` > based `java.net.Socket` implementation at that time had a bug where if > "ServerSocket.accept()" was blocked waiting for a connection and if the > native `accept()` call returned `EINTR` due to that system call being > terminated by a signal, then the Java application would end up incorrectly > receiving a `java.net.SocketTimeoutException: Accept timed out` exception. > That issued was fixed in JDK 16 and a couple of new jtreg tests were > introduced to verify the fix. This `SocketReadInterruptTest` was one of > those. Details of that fix and the test introduction are available in the RFR > email at that time: > > https://mail.openjdk.org/pipermail/net-dev/2020-March/013637.html > and > https://mail.openjdk.org/pipermail/net-dev/2020-March/013646.html > > The old `PlainSocketImpl` implementation of `java.net.Socket` is no longer > present in the JDK mainline. However, I think the idea is still a valid one > to verify that the Java application doesn't receive an unexpected exception > when it is blocked on `ServerSocket.accept()` or `Socket.read()` and the > `accept()` or `read()` system calls returned with `EINTR`. So the > `SocketReadInterruptTest` (and the `SocketAcceptInterruptTest`), I think are > still good to have in the JDK mainline. > > The `SocketReadInterruptTest` has been failing intermittently. What the test > does is, it creates a `ServerSocket`, then `accept()`s the connection from > the test's `Socket`. The test then initiates a `Socket.read()` with a socket > read timeout of 3 seconds. On the server side, the `ServerSocket` after > accepting the connection will wait for 2 seconds before writing out a > response on the accepted socket's outputstream. > > When the client side is waiting in `Socket.read()` (for a duration of 3 > seconds), the test sends out a signal to the native thread which is blocked > on the `read()` system call. This it does so that the `read()` system call > returns with a `EINTR`. The test then expects that no exception propagates to > the `Socket.read()` call and instead the `Socket.read()` completes normally. > This works fine in most cases. > > However, on certain occasions, if the server thread which is responsible for > writing out the response, is delayed ... test/jdk/java/net/Socket/SocketReadInterruptTest.java line 126: > 124: } catch (Throwable t) { > 125: System.err.println("Exception in client: " + t); > 126: t.printStackTrace(); Should we call `ready.countDown();` here too to unblock the main thread? test/jdk/java/net/Socket/SocketReadInterruptTest.java line 162: > 160: } > 161: return nativeThreadId; > 162: } An alternative could be to use a `CompletableFeature<Long>` instead of a CountDownLatch. test/jdk/java/net/Socket/SocketReadInterruptTest.java line 233: > 231: } > 232: } > 233: } Missing new line? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32051#discussion_r3691373648 PR Review Comment: https://git.openjdk.org/jdk/pull/32051#discussion_r3691387533 PR Review Comment: https://git.openjdk.org/jdk/pull/32051#discussion_r3691394692
