RFR: 8322829: Refactor nioBlocker to avoid blocking while holding Thread's interrupt lock

[Alan Bateman]

On Fri, 5 Jan 2024 17:27:30 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> In preparation for when virtual threads can unmount while holding a monitor or unmount when blocking on monitorenter, the implementation of VirtualThread's interrupt method is refactored to avoid parking/blocking while holding the Thread's interrupt lock. The implementations of sun.nio.ch.Interruptible are refactored to close/wakeup the InterruptibleChannel/Selector after releasing the interrupt lock. There is a lot of test coverage for async close and interrupt, no additional tests are added.
>
> src/java.base/share/classes/java/nio/channels/spi/AbstractInterruptibleChannel.java line 107:
> 
>> 105:             public void postInterrupt() {
>> 106:                 try {
>> 107:                     AbstractInterruptibleChannel.this.close();
> 
> Before the current change, the `Interruptible` implementation in this class used to call `AbstractInterruptibleChannel.this.implCloseChannel();` whereas now it calls `close()`. Is that an intentional change?

Yes, this is intentional.

> src/java.base/share/classes/java/nio/channels/spi/AbstractInterruptibleChannel.java line 180:
> 
>> 178:         Thread me = Thread.currentThread();
>> 179:         if (me.isInterrupted()) {
>> 180:             interruptor.interrupt(me);
> 
> The new javadoc comment on `Interruptor.interrupt(Thread)` states that "This method is invoked while holding the Thread's interrupt lock.", which isn't the case when being invoked from here.

This is an internal interface, I can re-phrase the method description to make it clear that this is when Thread.interrupt is called.

> src/java.base/share/classes/sun/nio/ch/Interruptible.java line 49:
> 
>> 47:      * Selector. This method is required to be idempotent.
>> 48:      */
>> 49:     void postInterrupt();
> 
> Should there be any thread safety note/expectations on this method now that it can be potentially called concurrently by multiple threads since it's called outside of the interrupt lock?

This is an internal interface, it's not a general purpose interface that anything outside of the NIO implementation should use. The phrase in the javadoc is meant to make it clear that it may be called more than once, and from several different I/O ops. on the channel.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17219#discussion_r1443153433
PR Review Comment: https://git.openjdk.org/jdk/pull/17219#discussion_r1443154022
PR Review Comment: https://git.openjdk.org/jdk/pull/17219#discussion_r1443155765