On Fri, 29 Nov 2024 11:38:05 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> Volkan Yazıcı has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - Use more fine-grained suppression
>>  - Further simplify tests
>>  - Extend `translateToSocketException()` with `AlreadyConnectedException`
>
> test/jdk/java/net/Socket/ConnectFailTest.java line 189:
> 
>> 187: 
>> 188:     @SuppressWarnings("ResultOfMethodCallIgnored")
>> 189:     private static void continuouslyAcceptConnections(ServerSocket 
>> serverSocket) {
> 
> I realize the test has gone through several rounds of simplification. I think 
> it can be simplified further as there is no need to use an ExecutorService or 
> have another thread managing a listen socket and accepting connections. The 
> reason is that all the supported operating systems queue accepted 
> connections. This means it can be all done synchronously, e.g. 
> testConnectedSocket and testConnectedSocketWithUnresolvedAddress can setup 
> the loopback connection with code like this:
> 
>         try (socket) {
>             withListener(listener -> {
>                 socket.connect(listener.getLocalSocketAddress());
>                 try (var peer = listener.accept()) {
>                     :
>                 }
>             });
>         }

That swiftly yielded 10 insertions and 56 deletions! 💥 (Implemented in 
33fad083d9b3b4a0ed753e2a9b179e9bbf87387c.)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1863538348

Reply via email to