RFR 8193879: Java debugger hangs on method invocation

Alex Menkov alexey.menkov at oracle.com
Thu Oct 4 22:53:23 UTC 2018


Looks good to me.

--alex

On 10/04/2018 15:13, Daniil Titov wrote:
> Hi Alex,
> 
> Just several minutes ago in another email I sent an updated patch on review (please see email attached). The patch has reimplemented tests and no longer uses JdbTest , so the changes you mentioned are no longer there. I also created issue https://bugs.openjdk.java.net/browse/JDK-8211736  to address the missing prompt problem.
> 
> Thanks!
> --Daniil
>   
> On 10/4/18, 2:53 PM, "Alex Menkov" <alexey.menkov at oracle.com> 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
>      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.")
>      
>      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".
>      
>      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