8383768: java/net/Socket/SocketReadInterruptTest.java failed with SocketTimeoutException: Read timed out

[jaikiran]

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.

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8383768: java/net/Socket/SocketReadInterruptTest.java failed with SocketTimeoutException: Read timed out (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/32051/head:pull/32051
$ git checkout pull/32051

Update a local copy of the PR:
$ git checkout pull/32051
$ git pull https://git.openjdk.org/jdk.git pull/32051/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 32051

View PR using the GUI difftool:
$ git pr show -t 32051

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/32051.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jpai! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@jaikiran The following label will be automatically applied to this pull request:

• net

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (02490832)