RFR 8193879: Java debugger hangs on method invocation

Thu Oct 4 22:13:17 UTC 2018

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
    >>>
    >>>
    >>>
    >>
    > 

-------------- next part --------------
An embedded message was scrubbed...
From: Daniil Titov <daniil.x.titov at oracle.com>
Subject: Re: RFR 8193879: Java debugger hangs on method invocation
Date: Thu, 04 Oct 2018 14:19:49 -0700
Size: 11231
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181004/4ac710de/attachment-0001.mht>