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

[Goetz Lindenmaier]

On Wed, 13 Sep 2023 13:04:44 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.
>> 
>> Thanks!
>
> Long Yang 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 10 additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8312065-fix
>  - use monotonically increasing time(System.nanoTime()) in place of currentTimeMillis
>  - use JNI_TRUE/JNI_FALSE to avoid introducing more ifdefs
>  - update comments
>  - Add a RETRY macro argument to avoid code duplication
>  - Check if SocketTimeoutException meets the specified time. Print detailed failure message.
>  - push to trigger github workflows
>  - Remove useless CountDownLatch
>  - use libNativeThread to get OS thread id and send signal
>  - 8312065: Socket.connect does not timeout as expected when profiling

You are still missing the fix request comment that states reason, risk and testing in the JBS bug.
You can issue another approval request (Use: approval request <text>" to do that, or edit JBS.  
For example see https://bugs.openjdk.org/browse/JDK-8232933

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

PR Comment: https://git.openjdk.org/jdk17u-dev/pull/1639#issuecomment-1719056227