Hi Daniil,
Thanks for the debug session. I did make me realize one flaw. The
"thread" command requires a thread number, but the breakpoint output
only includes the thread name. I wonder if we could also include the
thread number, although we'd have to be careful not to break any
existing tests that are parsing the output. I guess the other option for
the user is to use the "threads" command to map the thread name to a
thread number.
thanks,
Chris
On 10/12/18 10:30 AM, Daniil Titov wrote:
Hi Chris and Alex,
Please find below the output of the debug session:
===BEGIN===
jdb ThreadObj
Initializing jdb ...
stop thread at ThreadObj:4
Deferring breakpoint ThreadObj:4.
It will be set after the class is loaded.
run
run ThreadObj
Set uncaught java.lang.Throwable
Set deferred uncaught java.lang.Throwable
VM Started: Set deferred breakpoint ThreadObj:4
Breakpoint hit: "thread=main", ThreadObj.main(), line=4 bci=4
4 print(thread);
thread 1
main[1] cont
Thread[main,5,main]
The application exited
===END===
The following commands were entered:
1) stop thread at ThreadObj:4
2) run
3) thread 1
4) cont
We cannot call printCurrentLocation() for the case when the suspend policy is
set to SUSPEND_NONE, since in this case threadInfo.getCurrentFrame() does not
return the correct frame. Instead, for this case (when the suspend policy is
set to SUSPEND_NONE) I plan to print the breakpoint location using the
location returned by BreakPointEvent.location() method. I will also add the
new test for SUSPEND_NONE suspend policy case and address Alex's suggestions in
the new webrev I will send later.
Thanks,
Daniil
On 10/12/18, 9:52 AM, "Alex Menkov" <alexey.men...@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 <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
>>
>
