RFR 8211736: jdb doesn't print prompt when breakpoint is hit and suspend policy is STOP_EVENT_THREAD
Alex Menkov
alexey.menkov at oracle.com
Thu Oct 18 17:40:00 UTC 2018
+1
--alex
On 10/17/2018 18:53, JC Beyler wrote:
> Hi Daniil,
>
> It looks good to me.
>
> I noticed a tiny nit in TTY.java where you removed an empty line (l171)
>
> Thanks!
> Jc
>
>
> On Wed, Oct 17, 2018 at 5:50 PM Daniil Titov <daniil.x.titov at oracle.com
> <mailto:daniil.x.titov at oracle.com>> wrote:
>
> Hi Chris, Alex and JC,
>
> I created a separate issue to deal with "non-atomic" jdb output at
> https://bugs.openjdk.java.net/browse/JDK-8212626 .
>
> Please review an updated fix that includes the changes Alex suggested.
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8211736/webrev.04
> <http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.04>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8211736
>
> Thanks!
> --Daniil
>
>
> On 10/17/18, 5:06 PM, "Daniil Titov" <daniil.x.titov at oracle.com
> <mailto:daniil.x.titov at oracle.com>> wrote:
>
> Hi Chris,
>
> The previous email was accidentally fired before I managed to
> type anything in. Sorry for confusion.
>
> Jdb constantly reads new commands from System.in despite
> whether the breakpoint is hit and/or the prompt is printed.
>
> cat -n
> src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java
>
> 786 // Process interactive commands.
> 787 MessageOutput.printPrompt();
> 788 while (true) {
> 789 String ln = in.readLine();
> 790 if (ln == null) {
> 791 /*
> 792 * Jdb is being shutdown
> because debuggee exited, ignore any 'null'
> 793 * returned by readLine()
> during shutdown. JDK-8154144.
> 794 */
> 795 if (!isShuttingDown()) {
> 796
> MessageOutput.println("Input stream closed.");
> 797 }
> 798 ln = "quit";
> 799 }
> 800
> 801 if (ln.startsWith("!!") && lastLine
> != null) {
> 802 ln = lastLine + ln.substring(2);
> 803
> MessageOutput.printDirectln(ln);// Special case: use printDirectln()
> 804 }
> 805
> 806 StringTokenizer t = new
> StringTokenizer(ln);
> 807 if (t.hasMoreTokens()) {
> 808 lastLine = ln;
> 809 executeCommand(t);
> 810 } else {
> 811 MessageOutput.printPrompt();
> 812 }
> 813 }
> 814 } catch (VMDisconnectedException e) {
> 815 handler.handleDisconnectedException();
> 816 }
>
> Below is a sample debug session for the following test class
> that sends commands "threads" and "quit"
> 1 public class LoopTest {
> 2 public static void main(String[] args) throws
> Exception {
> 3 Thread thread = Thread.currentThread();
> 4 while(true) {
> 5 print(thread);
> 6 Thread.sleep(5000);
> 7 }
> 8 }
> 9
> 10 public static void print(Object obj) {
> 11 //System.out.println(obj);
> 12 }
> 13 }
>
> datitov-mac:work datitov$
> ~/src/jdk-hs/build/macosx-x64-debug/images/jdk/bin/jdb LoopTest
> Initializing jdb ...
> > stop go at LoopTest:6
> Deferring breakpoint LoopTest:6.
> It will be set after the class is loaded.
> > run
> run LoopTest
> Set uncaught java.lang.Throwable
> Set deferred uncaught java.lang.Throwable
> >
> VM Started: Set deferred breakpoint LoopTest:6
>
> Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
> 6 Thread.sleep(5000);
>
> Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
> 6 Thread.sleep(5000);
> threads
> Group system:
> (java.lang.ref.Reference$ReferenceHandler)0x172 Reference
> Handler running
> (java.lang.ref.Finalizer$FinalizerThread)0x173 Finalizer
> cond. waiting
> (java.lang.Thread)0x174 Signal
> Dispatcher running
> Group main:
> (java.lang.Thread)0x1 main
> sleeping
> Group InnocuousThreadGroup:
> (jdk.internal.misc.InnocuousThread)0x197
> Common-Cleaner cond. waiting
> >
> Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
> 6 Thread.sleep(5000);
>
> Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
> 6 Thread.sleep(5000);
> quit
> Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
> 6 Thread.sleep(5000);
>
> datitov-mac:work datitov$
>
> I think we could print a simple prompt in this case as Alex
> suggested.
>
> Best regards,
> Daniil
>
> On 10/17/18, 3:52 PM, "Chris Plummer"
> <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>> wrote:
>
> 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/
> <http://cr.openjdk.java.net/%7Edtitov/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 <mailto: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/>
> >>> >>
> <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
> <mailto:jcbeyler at google.com>>
> >>> >> *Date: *Thursday, October 11, 2018 at 7:17 PM
> >>> >> *To: *<daniil.x.titov at oracle.com
> <mailto:daniil.x.titov at oracle.com>>
> >>> >> *Cc: *<serviceability-dev at openjdk.java.net
> <mailto: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>
>
> >>>
> >>> >>
> >>>
> <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>
> >>> <mailto: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>
> >>> >>
> <http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.01>
> >>> >> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8211736
> >>> >>
> >>> >> Thanks!
> >>> >> --Daniil
> >>> >>
> >>> >>
> >>> >>
> >>> >> --
> >>> >>
> >>> >> Thanks,
> >>> >>
> >>> >> Jc
> >>> >>
> >>> >
> >>>
> >>>
> >>
> >>
>
>
>
>
>
>
>
> --
>
> Thanks,
> Jc
More information about the serviceability-dev
mailing list