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

[Daniel Fuchs]

On Tue, 3 Dec 2024 15:40:02 GMT, Tim Prinzing <tprinzing at openjdk.org> wrote:

>> Adds a JFR event for socket connect operations.
>> 
>> Existing tests TestSocketEvents and TestSocketChannelEvents modified to also check for connect events.
>
> Tim Prinzing has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits:
> 
>  - Merge branch 'master' into JDK-8310996
>    
>    # Conflicts:
>    #	src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java
>  - split socket connect failure out to its own event.
>  - 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
>  - suggested changes
>  - improved exception names and handling
>  - Added support for connection failure and non-blocking connections.
>    
>    If an exception is thrown while attempting a connection, the message
>    from the exception is stored in the event.  The start time of the
>    initial connect call is stored and used when a finishConnect call is
>    successful or an exception is thrown.
>  - fix default settings
>  - fix merge
>  - Merge branch 'master' into JDK-8310996
>    
>    # Conflicts:
>    #	src/jdk.jfr/share/classes/jdk/jfr/internal/JDKEvents.java
>    #	src/jdk.jfr/share/classes/jdk/jfr/internal/MirrorEvents.java
>  - added tests and support for sockets.
>  - ... and 1 more: https://git.openjdk.org/jdk/compare/dfa5620f...a379609e

test/jdk/jdk/jfr/event/io/TestSocketAdapterEvents.java line 149:

> 147:                 } catch (IOException ioe) {
> 148:                     // we expect this
> 149:                     connectException = ioe;

Though unlikely the connect() call could succeed if another (test) process managed to rebind and listen to the port that our server socket just freed. 
An alternative could be to try to connect to a TCP port reserved by IANA, such as ports 225-241 for instance. Hopefully there should be nothing listening at these ports.

Another possibility would be to restart the whole thing untill connect actually fails.

You could try to find a refusing port that will generate an exception using something similar to https://github.com/openjdk/jdk/blob/3eaa7615cd7dc67eb78fb0a8f89d4e6662a0db37/test/jdk/java/nio/channels/SocketChannel/OpenLeak.java#L65
Note that I had some trouble with that on windows (attempting to connect to port 47 was using up the whole connect timeout on windows) so you might need to experiment a bit with different reserved ports if you want to go down this route.
 
In anycase, the case where connect() unexpectedly succeed should be handled, for instance by throwing a `jtreg.SkippedException`, or by simply printing a warning and returning, or by retrying with another port until an exception is obtained.

test/jdk/jdk/jfr/event/io/TestSocketChannelEvents.java line 185:

> 183:                     // we expect this
> 184:                     connectException = ioe;
> 185:                 }

Same remark as previouly: it's not guaranteed that connect() will not succeed.
Also I notice that we're using main/othervm here - and this method is not last one called in main() - so we might not want to throw SkippedException here. Possibly printing a warning and returning would be a better option. Or try with another port...

test/jdk/jdk/jfr/event/io/TestSocketEvents.java line 146:

> 144:                     // we expect this
> 145:                     connectException = ioe;
> 146:                 }

Same remark as for the other test: exception is not a guaranteed outcome.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1867974742
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1867983998
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1868044001