RFR: 8323782: Race: Thread::interrupt vs. AbstractInterruptibleChannel.begin [v2]

[Richard Reingruber]

On Thu, 18 Jan 2024 08:32:02 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/Thread.java line 1709:
>> 
>>> 1707:         // Setting the interrupt status must be done before reading nioBlocker.
>>> 1708:         interrupted = true;
>>> 1709:         interrupt0();  // inform VM of interrupt
>> 
>> It is really safe/correct to move this outside the synchronized block? I know things have changed a bit with loom but we've "always" held a lock when doing the actual interrupt. I'd have to check the VM logic to be sure it can be called concurrently from multiple threads for the same target thread.
>
>> It is really safe/correct to move this outside the synchronized block? I know things have changed a bit with loom but we've "always" held a lock when doing the actual interrupt. I'd have to check the VM logic to be sure it can be called concurrently from multiple threads for the same target thread.
> 
> This hasn't changed. The interruptLock is used to coordinate the add/remove of the nioBlocker. When there is no nioBlocker set then the interrupt status and unparking (as in JavaThread::interrupt) has always executesd without the interruptLock.

I think that interrupting is just asynchronous to some extent.
E.g. a thread polls its interrupt status clearing it thereby (without lock) before calling nio. A concurrent interrupt can be lost then even if the lock is acquired.
(Maybe clearing should not be done by a public method)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17444#discussion_r1457152809