RFR: 8310996: Add JFR event for connect operations [v5]

[Daniel Fuchs]

On Sat, 23 Nov 2024 08:36:03 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Added more tests for socket connect events.
>>   
>>   - SocketAdapter connect
>>   - SocketAdapter connect with exception
>>   - Socket connect with exception
>>   - SocketChannel connect with exception
>>   - SocketChannel non-blocking connect
>>   - SocketChannel non-blocking connect with exception
>
> test/jdk/jdk/jfr/event/io/TestSocketChannelEvents.java line 157:
> 
>> 155:                         while (! sc.finishConnect()) {
>> 156:                             Thread.sleep(1);
>> 157:                         }
> 
> The connect method returns true if the connection is established, you should only need to poll finishConnect if the connection is not established immediately. Using a sleep is okay here (although 1ms is very low) and I'm guessing you've done this to avoid using a Selector.

It should be OK to sleep for longer if you don't want to use a selector.

> test/jdk/jdk/jfr/event/io/TestSocketChannelEvents.java line 216:
> 
>> 214:                 } catch (IOException ioe) {
>> 215:                     // we expect this
>> 216:                     connectException = ioe;
> 
> I think you'll need to adjust the try-catch to only set connectException if the connect or finishConnect methods fails. If the open or configureBlocking fails then connectException should not be set, instead the test should fail.

In addition there's no guarantee that connect will fail - so the test should guard for the case where connect might succeed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1867981816
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1868041972