Hi Gary,
It looks good to me.
Nice discovery!
Thanks,
Serguei
On 1/30/19 15:03, [email protected] wrote:
Second reviewer or is it trivial enough for one?
On 1/30/19 1:57 PM, Chris Plummer wrote:
This looks good.
thanks,
Chris
On 1/30/19 8:24 AM, Gary Adams wrote:
Following the trail from debugLoop_run, at the bottom of the loop
there is a path through debugInit_reset that involves
eventHandler_reset
and eventually eventHelper_reset. This seems like a better place to
clear the flag back to original state.
Revised webrev: http://cr.openjdk.java.net/~gadams/8215550/webrev.01/
On 1/29/19, 6:11 PM, Chris Plummer wrote:
Ok, so you can't do a "cont" because threads are not suspended.
That means someone resumed them. When/where was this done?
Regarding threadIDs changing, my guess is that debugLoop_run() is
re-entered when the new connection is established. This will result
in commonRef_reset() being called, which invalidates all reference
IDs, including threadIDs. So the first time the agent needs to send
a threadID to the debugger, it needs to create a new one.
Chris
On 1/29/19 1:52 PM, [email protected] wrote:
Issuing a "cont" in the second session does not work, because
the threads are not suspended.
It's interesting that the thread ids have all changed in
the reconnected session.
...
main[1] threads
Group system:
(java.lang.ref.Reference$ReferenceHandler)0x374 Reference
Handler running
(java.lang.ref.Finalizer$FinalizerThread)0x375 Finalizer cond.
waiting
(java.lang.Thread)0x376 Signal
Dispatcher running
Group main:
(java.lang.Thread)0x1 main running (at breakpoint)
Group InnocuousThreadGroup:
(jdk.internal.misc.InnocuousThread)0x377 Common-Cleaner cond.
waiting
main[1] exit
...
> threads
Group system:
(java.lang.ref.Reference$ReferenceHandler)0x3b2 Reference
Handler running
(java.lang.ref.Finalizer$FinalizerThread)0x3b3 Finalizer cond.
waiting
(java.lang.Thread)0x3b4 Signal
Dispatcher running
Group main:
(java.lang.Thread)0x3b7 main running
Group InnocuousThreadGroup:
(jdk.internal.misc.InnocuousThread)0x3b8 Common-Cleaner cond.
waiting
> cont
> Nothing suspended.
On 1/29/19 2:27 PM, Chris Plummer wrote:
What is the state of the threads after the detach? If they are
all automatically resumed by the agent, then I think the
unblocking should be done by the same code that resumes the
threads. If they are still suspended, then why would we want to
unblock when the next connection comes in? It should be up to the
debugger to detect that all threads are suspended and act
accordingly.
Also, what happens if after attaching again you issue a "cont"
command?
Chris
On 1/29/19 9:55 AM, Gary Adams wrote:
As far as I can tell, the quit and exit commands are only
handled locally
on the debugger side of the connection. There is no packet sent
to the
libjdwp agentlib.
On 1/29/19, 12:17 PM, Chris Plummer wrote:
Hi Gary,
When the "exit" or "quit" is done, aren't all threads resumed
at that point, and shouldn't that result in the command loop
being unblocked?
thanks,
Chris
On 1/29/19 8:09 AM, Gary Adams wrote:
Significant protections are put in place to protect the
commandLoop
from multiple events that that have a suspend-all policy. The
commandLoop uses a special block variable to ensure only
a VirtualMachine or ThreadReference call to resume() will unblock
the commandLoop. See
src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c
In this particular bug report, the user was stopped at a
breakpoint
when they sent the "exit" command. The same effect can be
produced
with a "quit" command or any killing of the debugger process.
When the second session is started the commandLoop is still
blocked,
so a new breakpoint will never be dequeued from the commandQueue.
The proposed workaround will ensure the commandLoop is unblocked
when a new session is started.
Issue: https://bugs.openjdk.java.net/browse/JDK-8215550
Webrev: http://cr.openjdk.java.net/~gadams/8215550/webrev.00/
All testing has been done by manually replicating the reported
command sequences. I'll see if an existing breakpoint test can be
enhanced to cover this scenario.
