Re: RFR 8211736: jdb doesn't print prompt when breakpoint is hit and suspend policy is STOP_EVENT_THREAD

Wed, 17 Oct 2018 15:54:01 -0700

What is the jdb state for a breakpoint with SUSPEND_NONE? Can you actually type in commands even though no threads are suspended? 

Chris

On 10/17/18 3:31 PM, Alex Menkov wrote:

Hi Daniil, Chris,
As far as I understand, the fix does not completely fixes all "non-atomic" output issues (at least TTY.executeCommand still prints prompt separately), so I agree that handle it as separate issue would be better. 
Unfortunately I don't know Gary's ideas for the issue.

About the fix for print prompt:
1) TTY.java:
+            // Print current location if suspend policy is SUSPEND_NONE
I suppose you mean "Print breakpoint location"
2) Does it make sense to use printCurrentLocation for SUSPEND_EVENT_THREAD? 
Is it expected to print something different from printBreakpointLocation?
3) I don't quite understand why we don't print simple prompt for SUSPEND_NONE. IIUC jdb can accept new command in both SUSPEND_EVENT_THREAD and SUSPEND_NONE cases (prompt shows that jdb waits for next command, right?) 

4) JdbStopThreadTest.java
New line line in Java regexp is "\\R"

--alex

On 10/17/2018 10:47, Chris Plummer wrote:

Hi Alex,
I think the tty buffering should be a separate bug fix, and I'd also like input from Gary on it since he had tried something similar at one point. It should probably also include a multithread test to prove the fix is working (after first getting the test to fail without the changes). 

thanks,

Chris

On 10/16/18 8:59 PM, Daniil Titov wrote:

Hi Alex, Chris and JC,
Please review an updated version of the patch that addresses these issues. 

Webrev: http://cr.openjdk.java.net/~dtitov/8211736/webrev.03/
Issue:  https://bugs.openjdk.java.net/browse/JDK-8211736

Thanks!
--Daniil


On 10/12/18, 9:52 AM, "Alex Menkov" <alexey.men...@oracle.com> wrote:

     Hi Daniil,
     1) +1 for printCurrentLocation when suspendPolicy != SUSPEND_ALL
     2) wrong indent in JdbStopThreadTest.java:
        36         import lib.jdb.JdbCommand;
        37         import lib.jdb.JdbTest;
     3) I think it would be better to make waitForSimplePrompt property of 
     JdbCommand (like JdbCommand.allowExit)
     jdb.command(JdbCommand.run().replyIsSimplePrompt());
     looks much clearer than
     jdb.command(JdbCommand.run(), true);
     4) (TTY.java)
         MessageOutput.lnprint("Breakpoint hit:");
     +  // Print current location and prompt if suspend policy is
     SUSPEND_EVENT_THREAD.
     +  // In case of SUSPEND_ALL policy this is handled by vmInterrupted() 
     method.
     +  if (be.request().suspendPolicy() == EventRequest.SUSPEND_EVENT_THREAD) { 
     + printCurrentLocation(ThreadInfo.getThreadInfo(be.thread()));
     +      MessageOutput.printPrompt();
     +  }
     We have 3 separate outputs.
     If we have several concurrent threads, we can easy get mess of outputs 
     from different threads.
     I think it would be better to print everything in a single chunk.
     I suppose TTY has other places with similar "non-atomic" output, so 
     maybe revising TTY output should be handled by separate issue.
     --alex
     On 10/11/2018 22:00, Chris Plummer wrote:
     > Hi Daniil,
     >
     > Can you send output from an example jdb session?
     >
     > Do you think maybe we should also call printCurrentLocation() when the 
     > suspend policy is NONE?
     >
     > There's a typo in the following line. The space is on the wrong side of 
     > the comma.
     >
     >    72 jdb.command(JdbCommand.stopThreadAt(DEBUGGEE_CLASS ,bpLine)); 
     >
     > thanks,
     >
     > Chris
     >
     > On 10/11/18 8:02 PM, Daniil Titov wrote:
     >>
     >> Thank you,  JC!
     >>
     >> Please review an updated version of the patch that fixes newly added 
     >> test for Windows platform  (now it uses system dependent line
     >> separator string).
     >>
     >> Webrev:http://cr.openjdk.java.net/~dtitov/8211736/webrev.02/
     >> <http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.02/>
     >>
     >> Issue: https://bugs.openjdk.java.net/browse/JDK-8211736
     >>
     >> Best regards,
     >>
     >> --Daniil
     >>
     >> *From: *JC Beyler <jcbey...@google.com>
     >> *Date: *Thursday, October 11, 2018 at 7:17 PM
     >> *To: *<daniil.x.ti...@oracle.com>
     >> *Cc: *<serviceability-dev@openjdk.java.net>
     >> *Subject: *Re: RFR 8211736: jdb doesn't print prompt when breakpoint 
     >> is hit and suspend policy is STOP_EVENT_THREAD
     >>
     >> Hi Daniil,
     >>
     >> Looks good to me. I saw a really small nit:
     >>
     >> http://cr.openjdk.java.net/~dtitov/8211736/webrev.01/test/jdk/com/sun/jdi/JdbStopThreadTest.java.html      >> <http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.01/test/jdk/com/sun/jdi/JdbStopThreadTest.java.html> 
     >>
     >> 70  jdb.command(JdbCommand.stopThreadAt( DEBUGGEE_CLASS ,bpLine)); 
     >>
     >> There is a space after the '('.
     >>
     >> No need to send a new webrev for that evidently :),
     >>
     >> Jc
     >>
     >> On Thu, Oct 11, 2018 at 5:07 PM Daniil Titov
     >> <daniil.x.ti...@oracle.com <mailto:daniil.x.ti...@oracle.com>> wrote: 
     >>
     >>     Please review the change that fixes the issue with missing prompt      >>     in jdb when a breakpoint is hit and the suspend policy is set to 
     >>     stop the thread only.
     >>
     >>     Webrev: http://cr.openjdk.java.net/~dtitov/8211736/webrev.01 
     >> <http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.01>
     >>     Issue: https://bugs.openjdk.java.net/browse/JDK-8211736
     >>
     >>     Thanks!
     >>     --Daniil
     >>
     >>
     >>
     >> --
     >>
     >> Thanks,
     >>
     >> Jc
     >>
     >

Reply via email to