[jdk17u-dev] RFR: 8312065: Socket.connect does not timeout when profiling

[Andrei Pangin]

On Fri, 4 Aug 2023 03:02:00 GMT, Long Yang <duke at openjdk.org> wrote:

> Hi all,
> 
> This pull request contains a fix for [JDK-8312065](https://bugs.openjdk.org/browse/JDK-8312065).
> 
> The old SocketImpl is still present in JDK 17, can be enabled with -Djdk.net.usePlainSocketImpl.
> 
> I have verified that this problem exists on Linux and macOS, and I feel that it also exists on AIX, 
> so I repaired these 3 platforms. Windows implementations are different, so don't have this problem.
> 
> All callers of NET_Poll() already check for EINTR, so the fix is to remove do-while loop in the implementation of NET_Poll().
> 
> Other methods, such as NET_Read, NET_NonBlockingRead, NET_Accept, NET_Connect, are not affected.
> Because if Socket has timeout, it will call NET_Timeout first, and then call them. NET_Timeout has a correct timeout implementation and is not affected by profiling signals.
> If the Socket does not have a timeout, and if the system call is interrupted by a signal, the [Linux Kernel will automatically retry](https://man7.org/linux/man-pages/man7/signal.7.html), and the application code will not be aware of it.
> 
> I added a platform dependent test.
> 
> The test needs to get the OS thread ID of the Java thread. It seems that there is no good Java API to get it directly. The minimum requirement of Linux [/proc/thread-self](https://man7.org/linux/man-pages/man5/proc.5.html#:~:text=/proc/thread%2Dself%20(since%20Linux%203.17)) is Linux 3.17.
> The JFR event records the OS thread ID, although this method is cumbersome, it is reliable.
> 
> The test also needs to use the kill command to send a signal to the thread. macOS's kill seems to only support process, and I don't have an AIX environment, so this test only supports Linux.
> 
> Thanks!

test/jdk/java/net/Socket/B8312065.java line 181:

> 179:      */
> 180:     private static long getOSThreadId() throws IOException, ParseException {
> 181:         startJFR();

You may reuse `libNativeThread` for finding threadId and sending signals. This should make the test a lot simpler.
See `SocketAcceptInterruptTest` and `SocketReadInterruptTest` for example.
JFR is an optional feature, it is not available in Minimal VM.

test/jdk/java/net/Socket/B8312065.java line 249:

> 247:         success = true;
> 248:     }
> 249: }

EOL is missing on the last line

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

PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/1639#discussion_r1300146121
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/1639#discussion_r1300147412