On Tue, 3 Jun 2025 12:56:49 GMT, Alan Bateman <al...@openjdk.org> wrote:
> If several threads attempt to read from a Socket's input stream at the same
> time then all but the winner will block trying to acquire the read lock.
> This is okay for untimed-reads but surprising for timed-reads as the timeout
> is only effective after acquiring the lock. The SocketImpl is changed so that
> the timeout applies to the total time waiting to acquire and read.
>
> A new test is added to the existing java/net/Socket/Timeouts test. It is
> migrated from TestNG to a JUnit test as a drive-by change - it's mostly
> mechanical and the changes kept as minimal as possible.
Changes LGTM. Minor comment on the test.
test/jdk/java/net/Socket/Timeouts.java line 80:
> 78: try (Socket s = new Socket()) {
> 79: SocketAddress remote = Utils.refusingEndpoint();
> 80: assertThrows(ConnectException.class, () -> s.connect(remote,
> 10000));
Ah! Good - you're even fixing a bug here.
test/jdk/java/net/Socket/Timeouts.java line 249:
> 247: () -> s.getInputStream().read());
> 248: int timeout = s.getSoTimeout();
> 249: checkDuration(startMillis, timeout-100, timeout+20_000);
Maybe it would be worth noting here that if the bug isn't fixed, the last
thread to call read() would have to wait for 200 seconds, which largely exceeds
22s?
-------------
Marked as reviewed by dfuchs (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/25614#pullrequestreview-2893023129
PR Review Comment: https://git.openjdk.org/jdk/pull/25614#discussion_r2124221517
PR Review Comment: https://git.openjdk.org/jdk/pull/25614#discussion_r2124218812
