On Thu, 18 Jun 2026 23:33:31 GMT, Man Cao <[email protected]> wrote:
> Hi all,
>
> Could anyone review this change contributed by colleague Liz Looney
> <[email protected]>, that fixes a long-standing data race on
> `com.sun.tools.jdi.VirtualMachineImpl.shutdown`?
>
> -Man
>
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK
> Interim AI Policy](https://openjdk.org/legal/ai).
I've been coincidentally working in this code for #31421 and the related #31586
PR, and I've uncovered other issues with processBatchedDisposes() during
shutdown/disconnect.
I think the purpose of the shutdown check is to avoid getting a
VMDisconnectedException when the JDWP.VirtualMachine.DisposeObjects.process()
line is executed. However, the check is not enough since shutdown can still be
set true after the check is made and before process() is called. We really need
some sort of synchronzation that includes shutdown and the process() call.
However, that still is not enough to prevent VMDisconnectedException from
happening here. VirtualMachineImpl.objectMirror() calls processQueue().
processQueue() was modified by JDK-8297638 in 2023 to call
processBatchedDisposes() if any unreferenced ObjectReferenceImpls were found.
processBatchedDisposes() can throw VMDisconnectedException. See the stack trace
at the end of this comment.
The issue here is the JDI user has called EventQueue.remove(), and in this case
should be getting a ClassPrepareEvent, and since the debuggee has already
disconnected by this time, it should be followed by a VMDisconnectEvent. These
should be delivered without producing a VMDisconnectedException. But due to the
processBatchedDisposes() call now being in the path, a VMDisconnectedException
is being thrown while trying to send a VirtualMachine.DisposeObjects.
I'm seeing this issue a lot in my testing, but only after modifying the
referenceQueue used to track ObjectReferences to use weakrefs instead of
softrefs (so they are more readily collected), and also making it so
VirtualMachine.DisposeObjects is called if there are any batched disposes
instead of waiting until there are 50. This makes the scenario described above
much more likely to happen since VirtualMachine.DisposeObjects is executed much
more readily and frequently.
I think the fix is as simple as catching VMDisconnectedException around the
process() call and ignoring it. If the debuggee is disconnected, the ObjectIDs
are already explicity disposed of, and the VMDisconnectedException can be
ignored. This allows the debugger to fetch the last remaining events that are
already queued, including VMDisconnectEvent and VMDeathEvent. I think that will
also allow us to get rid of the shudown related logic. Even if we've already
done a dispose() or exit(), it is safe to attempt the
VirtualMachine.DisposeObjects and allow it to fail with VMDisconnectedException.
Also see the comment in validateVM(), which seems to support this approach. If
this change is made to ignore VMDisconnectedException, then we don't need the
shutdown check. I'm still working on this fix. Testing turned up some test
issue that I'm still trying to resolve.
com.sun.jdi.VMDisconnectedException
at jdk.jdi/com.sun.tools.jdi.TargetVM.waitForReply(TargetVM.java:305)
at
jdk.jdi/com.sun.tools.jdi.VirtualMachineImpl.waitForTargetReply(VirtualMachineImpl.java:1174)
at
jdk.jdi/com.sun.tools.jdi.PacketStream.waitForReply(PacketStream.java:89)
at
jdk.jdi/com.sun.tools.jdi.JDWP$VirtualMachine$DisposeObjects.waitForReply(JDWP.java:1004)
at
jdk.jdi/com.sun.tools.jdi.JDWP$VirtualMachine$DisposeObjects.process(JDWP.java:979)
at
jdk.jdi/com.sun.tools.jdi.VirtualMachineImpl.processBatchedDisposes(VirtualMachineImpl.java:1351)
at
jdk.jdi/com.sun.tools.jdi.VirtualMachineImpl.processQueue(VirtualMachineImpl.java:1384)
at
jdk.jdi/com.sun.tools.jdi.VirtualMachineImpl.objectMirror(VirtualMachineImpl.java:1391)
at
jdk.jdi/com.sun.tools.jdi.VirtualMachineImpl.threadMirror(VirtualMachineImpl.java:1479)
at
jdk.jdi/com.sun.tools.jdi.PacketStream.readThreadReference(PacketStream.java:457)
at
jdk.jdi/com.sun.tools.jdi.JDWP$Event$Composite$Events$ClassPrepare.<init>(JDWP.java:8627)
at
jdk.jdi/com.sun.tools.jdi.JDWP$Event$Composite$Events.<init>(JDWP.java:7872)
at jdk.jdi/com.sun.tools.jdi.JDWP$Event$Composite.<init>(JDWP.java:8901)
at jdk.jdi/com.sun.tools.jdi.EventSetImpl.build(EventSetImpl.java:660)
at
jdk.jdi/com.sun.tools.jdi.EventQueueImpl.removeUnfiltered(EventQueueImpl.java:212)
at
jdk.jdi/com.sun.tools.jdi.EventQueueImpl.remove(EventQueueImpl.java:97)
at nsk.share.jdi.JDIBase.getEventSet(JDIBase.java:169)
at
nsk.jdi.StepRequest.addInstanceFilter.instancefilter003.runTest(instancefilter003.java:211)
at
nsk.jdi.StepRequest.addInstanceFilter.instancefilter003.runThis(instancefilter003.java:151)
at
nsk.jdi.StepRequest.addInstanceFilter.instancefilter003.run(instancefilter003.java:94)
at
nsk.jdi.StepRequest.addInstanceFilter.instancefilter003.main(instancefilter003.java:85)
at
java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:583)
at
com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:335)
at java.base/java.lang.Thread.run(Thread.java:1527)
-------------
PR Comment: https://git.openjdk.org/jdk/pull/31582#issuecomment-4770681842
