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

Reply via email to