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 for longer than the 3 second client read duration, and as a result cannot write out the response, then the client side `Socket.read()` ends up with a `SocketTimeoutException: Read timed out` as seen in these intermittent failures. The fix in this PR addresses the test issue by changing the socket read timeout from 3 seconds to a very large timeout. This will allow the test to exercise the timed read of the `java.net.Socket` implementation and at the same time it should give enough time for the server side to be able to write out the response. Additional changes have been done to make the test more robust as well as easier to understand and maintain. With these changes the test has been run in our CI with a test-repeat of 50 and has also in tier2. No failures have been noticed with this change. Note that it's likely that the other test `SocketAcceptInterruptTest` might need a general clean up too, but I decided not to do it for now. --------- - [x] I confirm that I make this contribution in accordance with the [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). ------------- Commit messages: - 8383768: java/net/Socket/SocketReadInterruptTest.java failed with SocketTimeoutException: Read timed out Changes: https://git.openjdk.org/jdk/pull/32051/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=32051&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8383768 Stats: 161 lines in 1 file changed: 72 ins; 18 del; 71 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
