On Fri, 5 Jan 2024 17:27:30 GMT, Jaikiran Pai <j...@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
