On Mon, 4 Mar 2024 11:51:01 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Alan and David, thank you for the comments! >> The initial implementation of the `is_iterrupt()` intentionally avoided any >> synchronization and allowed a potential raises with the concurrent >> interrupts. Please, see the comment above at lines: `573-587`. It is why >> David mentioned the need to update the comment. I agree with Alan that that >> interrupt status of virtual and carrier threads have to be kept in sync. But >> as we already discussed with Alan before, an upcall to Java does not looks >> good and can cause some issues on the JDWP side. I'm thinking to model a CAS >> kind of operation to keep the two interrupt statuses in sync but still need >> to prove it is going to be safe. Also, there is a question if it worth the >> complexity. >> The need to keep interrupt status in both virtual and carrier threads comes >> from virtual thread pinning. I believe, there would be no need to keep >> interrupt status for both threads in the Java object monitors >> inplementations. AFAICS, then the implementation of the `is_interrupted()` >> can be kept as it is now. >> So, it looks like a good suggestion to wait for Java object monitors. >> However, the JDWP related fix depends on this and will need to wait as well. > > David can correct me, but I think the only cases in the VM where a thread may > clear its interrupt status is Thread.interrupted, Thread.sleep, Object.wait, > and JVMTI RawMonitorWait. Thread.interrupted and Thread.sleep are different > implementation and not interesting here. Right now, Object.wait does wait on > the carrier. If the virtual thread is interrupted while in the monitor's wait > set then InterruptedException will be thrown when the thread reenters and the > interrupt status of both threads will be cleared at that point (it will have > of course have been cleared by the VM too prior to throwing). If something > sneaky were to go behind our backs and interrupt the carrier directly then it > will be benign for this case but might cause a spurious InterruptedException > at other times, say where the mounted virtual thread were to call > Object.wait soon after its carrier was interrupted directly. We've mostly > ignored that concern. > > For JVMTI RawMonitorWait then it has to coordinate with Thread.interrupt and > JVMTI InterruptThread. The latter does do an upcall when the target is a > virtual thread so it's the same as Thread.interrupt. So minimally > RawMonitorWait will need to disable suspend and and clear the interrupt > status of both threads while holding the interrupt lock. @AlanBateman said: > So minimally RawMonitorWait will need to disable suspend and and clear the > interrupt status of both threads while holding the interrupt lock. If we do it with a Java upcall to the `VirtualThread.getAndClearInterrupt()` then we also have to skip the JVMTI events, AFAIK, as it is not a good idea to post the JVMTI events recursively. This is going to touch many spots in the `jvmtiExport.cpp`. Otherwise, I do not see how to hold the interruptLock from the VM side. Any advices? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18093#discussion_r1511933508