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

[Chris Plummer]

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