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