RFR 8193879: Java debugger hangs on method invocation
Chris Plummer
chris.plummer at oracle.com
Fri Oct 5 04:52:44 UTC 2018
On 10/4/18 6:00 PM, Alex Menkov wrote:
>
>
> On 10/04/2018 16:11, Chris Plummer wrote:
>> On 10/4/18 2:53 PM, Alex Menkov wrote:
>>>
>>>
>>> On 10/04/2018 10:28, Chris Plummer wrote:
>>>> On 10/4/18 5:12 AM, Gary Adams wrote:
>>>>> In TTY.java do you need to force a simple prompt for the
>>>>> breakpoint event output? What prevents currentThread from
>>>>> being set at the time you are printing the prompt?
>>>>>
>>>>>
>>>>> 103 // Print the prompt if suspend policy is
>>>>> SUSPEND_EVENT_THREAD. In case of
>>>>> 104 // SUSPEND_ALL policy this is handled by
>>>>> vmInterrupted() method.
>>>>> 105 if (be.request().suspendPolicy() ==
>>>>> EventRequest.SUSPEND_EVENT_THREAD) {
>>>>> 106 MessageOutput.println();
>>>>> 107 MessageOutput.printPrompt();
>>>> Normally the thread is suspended after the above code is executed:
>>>>
>>>> public void run() {
>>>> EventQueue queue = Env.vm().eventQueue();
>>>> while (connected) {
>>>> try {
>>>> EventSet eventSet = queue.remove();
>>>> boolean resumeStoppedApp = false;
>>>> EventIterator it = eventSet.eventIterator();
>>>> while (it.hasNext()) {
>>>> resumeStoppedApp |=
>>>> !handleEvent(it.nextEvent()); <--- calls the modified code above
>>>> }
>>>>
>>>> if (resumeStoppedApp) {
>>>> eventSet.resume();
>>>> } else if (eventSet.suspendPolicy() ==
>>>> EventRequest.SUSPEND_ALL) {
>>>> setCurrentThread(eventSet); <------
>>>> notifier.vmInterrupted();
>>>> }
>>>>
>>>> However, it only calls setCurrentThread() for SUSPEND_ALL policies.
>>>> So what Daniil has done here is make it print a simple prompt if
>>>> the policy is SUSPEND_EVENT_THREAD. It's unclear to me what the
>>>> actual debugger behavior is in this case. Don't we still suspend
>>>> and get a prompt from which we can type in the next command? In
>>>> this case, wouldn't you want a full prompt? Related to that
>>>> question, why is vmInterrupted() only called if we suspend all
>>>> threads, and not when we just suspend the thread that the
>>>> breakpoint came in on?
>>>
>>> Did a quick test
>>> - run debuggee (suspended), connect jdb:
>>> ===========
>>> VM Started: No frames on the current call stack
>>>
>>> main[1] stop thread at myclass:71
>> I don't see "stop thread at" in the jdb help output. I assume it
>> means the same as "stop at", but only suspends the current thread.
>
> Yes, you're right. But the bug has "stop thread at" in the testcase.
> Actually stop at can be "stop go at ..." which means the policy is set
> to SUSPEND_NONE.
> This looks quite funny - breakpoint which doesn't break the execution :)
I guess that's like a tracing feature, but it doesn't seem to tell you
where you hit the breakpoint, only that one was hit, which isn't all
that useful. I think we should call printCurrentLocation() for any
breakpoint being hit, but we only do that for SUSPEND_ALL.
>
>>
>>> Deferring breakpoint myclass:71.
>>> It will be set after the class is loaded.
>>> main[1] run
>>> > Set deferred breakpoint myclass:71
>>>
>>> Breakpoint hit:
>>> =====
>>> I.e. we get only "Breakpoint hit: " without any info about the
>>> thread (and this doesn't look good), current thread is not set
>>> ("next" reports "Nothing suspended.")
>> Where is the input cursor at this point? Right after "Breakpoint
>> hit:" or on the next (empty) line.
>
> After "Breakpoint hit: " (space after the colon)
So you don't even know which thread you hit the breakpoint in. Another
reason calling printCurrentLocation() would be useful.
>
>>>
>>> But if we handle SUSPEND_EVENT_THREAD like SUSPEND_ALL, we need to
>>> think how to handle possible race conditions when there are 2
>>> threads, each of them is stopped by "stop thread at/in".
>> Well, not really a race condition, but after hitting the breakpoint
>> in one thread, you could hit the same breakpoint again in another
>> thread. So you are in this state where you are both at the user
>> command prompt, and new events can still come in. Also, now you have
>> two threads suspended. I suppose to resume a thread you need to
>> switch to it with "thread" and then execute "cont".
>
> Yes, you are right.
> Considering this scenario set current thread for SUSPEND_EVENT_THREAD
> requests doesn't look good.
If we did set the current thread, you could then have a race condition
with the jdb user. You hit the breakpoint, type in "cont", but another
breakpoint comes in just before you press <enter>. You end up continuing
the second thread in that case instead of the first as you intended.
Chris
>
> --alex
>
>>
>> Chris
>>>
>>> Some notes about the fix (actually about the tests as the fix itself
>>> should be implemented):
>>> JdbStopThreadWithTraceOnTest.java
>>> 70 jdb.command("stop thread at " + DEBUGGEE_CLASS + ":" +
>>> bpLine);
>>>
>>> It would be better to introduce
>>> public static JdbCommand JdbCommand.stopThreadAt(String targetClass,
>>> int lineNum)
>>> Actually may be it would be better to drop Jdb.command(String) -
>>> it's required only by CommandCommentDelimiter (need to update it to
>>> use "new JdbCommand(...)").
>>>
>>> If the fix will require "simple prompt" reply, I'd make it
>>> JdbCommand property (like JdbCommand.allowExit)
>>>
>>> --alex
>>>
>>>>
>>>> Chris
>>>>>
>>>>>
>>>>> In Jdb.java you allow the waiting for the simple prompt.
>>>>> I don't see waitForSimplePrompt used in any existing tests.
>>>>> Since it is only looking for a single character it could
>>>>> produce false positives if the '>' was produced in the
>>>>> output stream. Is this wait paired to the added prompt for the
>>>>> break point event?
>>>>>
>>>>> 236 return waitForSimplePrompt ? waitForSimplePrompt(1,
>>>>> cmd.allowExit) : waitForPrompt(1, cmd.allowExit);
>>>>>
>>>>> It might be a good idea to include a test with multiple
>>>>> threads each with a breakpoint that will trigger SUSPEND_EVENT_THREAD
>>>>> behavior.
>>>>>
>>>>> On 10/4/18, 12:29 AM, Daniil Titov wrote:
>>>>>> Please review the changes that fix the deadlock in the debugger
>>>>>> when the debugger is running with the tracing option on.
>>>>>>
>>>>>> The problem here is that when the tracing is on the "JDI Target
>>>>>> VM Interface" thread (the thread that reads all replies and then
>>>>>> notifies the thread that sent the request that the reply has been
>>>>>> received) is waiting for a lock which is already acquired by the
>>>>>> thread that sent the request and is waiting for reply.
>>>>>>
>>>>>> The fix itself is in
>>>>>> src/jdk.jdi/share/classes/com/sun/tools/jdi/VMState.java.
>>>>>>
>>>>>> The patch also reverts the changes done in 8129348 "Debugger
>>>>>> hangs in trace mode with TRACE_SENDS" in
>>>>>> src/jdk.jdi/share/classes/com/sun/tools/jdi/InvokableTypeImpl.java
>>>>>> since they address only a specific case (VM is suspended and
>>>>>> static method is invoked) while the proposed fix also solves
>>>>>> issue 8129348 as well as issue 8193801 "Debugger hangs in trace
>>>>>> mode for non-static methods".
>>>>>>
>>>>>> The changes include new tests for issues 8193879, 8193801 and
>>>>>> 8129348.
>>>>>>
>>>>>> The changes in
>>>>>> src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java
>>>>>> solve the problem that the prompt is not printed in the debugger
>>>>>> output when the breakpoint is hit and the suspend policy is
>>>>>> SUSPEND_EVENT_THREAD. This is required for new tests to detect
>>>>>> that command "stop thread at ..." is completed.
>>>>>>
>>>>>> Mach5 build and jdk_jdi tests succeeded.
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~dtitov/8193879/webrev.01/
>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8193879
>>>>>>
>>>>>> Thanks,
>>>>>> --Daniil
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>
More information about the serviceability-dev
mailing list