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

[Daniil Titov]

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.menkov at 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 <jcbeyler at google.com>
    >> *Date: *Thursday, October 11, 2018 at 7:17 PM
    >> *To: *<daniil.x.titov at oracle.com>
    >> *Cc: *<serviceability-dev at 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.titov at oracle.com <mailto:daniil.x.titov at 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
    >>
    >