> 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 ...
Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision: - Add a comment to clarify a test class method - merge latest from master branch - Daniel's suggestion - use CompletableFuture instead of CountDownLatch - add newline add end of file - merge latest from master branch - 8383768: java/net/Socket/SocketReadInterruptTest.java failed with SocketTimeoutException: Read timed out ------------- Changes: - all: https://git.openjdk.org/jdk/pull/32051/files - new: https://git.openjdk.org/jdk/pull/32051/files/6e3900c9..5ef610f4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=32051&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=32051&range=01-02 Stats: 6166 lines in 119 files changed: 3804 ins; 2116 del; 246 mod Patch: https://git.openjdk.org/jdk/pull/32051.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/32051/head:pull/32051 PR: https://git.openjdk.org/jdk/pull/32051
