On Fri, 31 Jul 2026 15:08:43 GMT, Daniel Fuchs <[email protected]> wrote:

>> 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 four additional 
>> commits since the last revision:
>> 
>>  - 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
>
> 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?

Good catch. Addressed in the latest update of this PR.

> 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.

Hello Daniel, yes I think that's a cleaner option that using a CountDownLatch. 
I've updated the PR accordingly. The test continues to pass.

> test/jdk/java/net/Socket/SocketReadInterruptTest.java line 233:
> 
>> 231:         }
>> 232:     }
>> 233: }
> 
> Missing new line?

Fixed.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/32051#discussion_r3706150499
PR Review Comment: https://git.openjdk.org/jdk/pull/32051#discussion_r3706146361
PR Review Comment: https://git.openjdk.org/jdk/pull/32051#discussion_r3706142878

Reply via email to