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
Fri Oct 19 17:20:15 UTC 2018
Hi Gary,
Just want to remind that jdb is a tool for developers, not a tool for
testing.
On 10/19/2018 00:54, gary.adams at oracle.com wrote:
> It's not clear to me if the omitted location information for the non
> stopping
> cases was intentional or not. It may be sufficient to know that the
> event fired
> without the extra information.
A single breakpoint is not a common case. Usually you have a number of
breakpoints in the debuggee.
Note also that currently jdb prints "Breakpoint hit: " only (and colon
at the end shows that extra info is expected to be printed).
>
> I don't think a settable prompt is required either. There are plenty of
> jdb commands that
> could be used with the wait for message in tests that need to
> synchronize at a specific
> point in the test sequence.
Once again - this is a tool for humans, and show prompt when user input
is expected is a good UI practice.
--alex
>
> I don't think we see wait for simple prompt in any of the tests, because it
> is not reliable enough.
>
> On 10/18/18 11:53 AM, Daniil Titov wrote:
>> Hi Gary,
>>
>> Currently when a breakpoint is hit the message "Breakpoint hit:" is
>> printed in the debugger output. What we do in this fix we just add
>> more information about what exact breakpoint is hit. Do you suggest we
>> should not print this line at all if suspend policy is SUSPEND_NONE?
>> If so then it is not clear what is the use of the command "stop go
>> ..." would be. Regarding waiting for the simple prompt, we could
>> change JDbCommand to have a field to store a prompt pattern and use
>> this pattern (if it was set) when waiting for command to complete. In
>> tests when required we would set the pattern in JdbCommand to more
>> complicated one matching a specific output we are expecting at this
>> particular step. Do you think it would be a better approach?
>>
>> Thanks!
>>
>> Best regards,
>> Daniil
>>
>>
>>
>> On 10/18/18, 4:09 AM, "Gary Adams" <gary.adams at oracle.com> wrote:
>>
>> I'm not certain that we should be printing locations or prompts for
>> events when the vm has not been suspended. This looks OK for the
>> simple test case we are working on here, but in real life there may
>> be a lot more output produced.
>> The user has to select a specific thread before additional commands
>> can be performed in the correct context. e.g. threads, thread n,
>> where, ...
>> So the information is available to the user. It doesn't have to be
>> produced at the time the event is processed.
>> I'm uncomfortable putting too much trust in waiting for the
>> simple prompt,
>> because there is too great a chance of false positives on such a
>> small
>> marker.
>> On 10/17/18, 8:50 PM, Daniil Titov 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
>> > 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> 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> 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/
>> > >>> 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
>> > >>> >>
>> > >>> >
>> > >>>
>> > >>>
>> > >>
>> > >>
>> >
>> >
>> >
>> >
>> >
>>
>>
>
More information about the serviceability-dev
mailing list