JDK-8219143: jdb should support breakpoint thread filters

[Chris Plummer]

Please review the updated webrev:

http://cr.openjdk.java.net/~cjplummer/8219143/webrev.01/

The syntax is now:

     stop [go|thread] [threadid <thread_id>] <at|in> <location>

I filed JDK-8219507 to cover BreakpointSpec.toString() improvements, and 
other possible improvements to the output when listing breakpoints.

I did a minor fix in TTYResources.java to address a bug from a recent 
JDK-8218941 commit that added the dbgtrace command. There was missing 
newline. I've added a note to JDK-8218941 indicating that it is being 
fixed here.

I've added a test. Below is a log of the debug session output that might 
help with reading the source of the test. The test creates 3 threads 
that all execute the same code, and verifies that the breakpoint set 
using the threadid option only stops on the one specified thread.

thanks,

Chris

[jdb] Set uncaught java.lang.Throwable
[jdb] Set deferred uncaught java.lang.Throwable
[jdb] Initializing jdb ...
[jdb]
[jdb] VM Started: > No frames on the current call stack
[jdb]
[jdb] main[1]
 > stop in JdbStopThreadidTestTarg.brkMethod
[jdb] Deferring breakpoint JdbStopThreadidTestTarg.brkMethod.
[jdb] It will be set after the class is loaded.
[jdb] main[1]
 > run
[jdb] > Set deferred breakpoint JdbStopThreadidTestTarg.brkMethod
[jdb]
[jdb] Breakpoint hit: "thread=main", 
JdbStopThreadidTestTarg.brkMethod(), line=80 bci=0
[jdb] 80        }
[jdb]
[jdb] main[1]
 > threads
[jdb] Group system:
[jdb]   (java.lang.ref.Reference$ReferenceHandler)367 Reference Handler 
running
[jdb]   (java.lang.ref.Finalizer$FinalizerThread)368 Finalizer         
cond. waiting
[jdb]   (java.lang.Thread)369                         Signal Dispatcher 
running
[jdb] Group main:
[jdb]   (java.lang.Thread)1 main              running (at breakpoint)
[jdb]   (JdbStopThreadidTestTarg$MyThread)482 MYTHREAD-1        waiting 
in a monitor
[jdb]   (JdbStopThreadidTestTarg$MyThread)483 MYTHREAD-2        waiting 
in a monitor
[jdb]   (JdbStopThreadidTestTarg$MyThread)484 MYTHREAD-3        waiting 
in a monitor
[jdb] Group InnocuousThreadGroup:
[jdb]   (jdk.internal.misc.InnocuousThread)416 Common-Cleaner    cond. 
waiting
[jdb] main[1]
 > stop threadid 483 in JdbStopThreadidTestTarg$MyThread.brkMethod
[jdb] Set breakpoint JdbStopThreadidTestTarg$MyThread.brkMethod
[jdb] main[1]
 > cont
[jdb] >
[jdb] Breakpoint hit: "thread=MYTHREAD-2", 
JdbStopThreadidTestTarg$MyThread.brkMethod(), line=101 bci=0
[jdb] 101            }
[jdb]
[jdb] MYTHREAD-2[1]
 > cont
[jdb] >
[jdb] The application exited
[jdb]


On 2/20/19 2:23 PM, Chris Plummer wrote:
> On 2/20/19 2:00 PM, Alex Menkov wrote:
>> Hi Chris,
>>
>> - New threadid param breaks at/in clause - this doesn't look good.
>> Maybe it would be better to put threadid before in/at:
>> stop [go|thread] [threadid <thread_id>] <at|in> <location>
> Ok. I'll try moving it.
>> This also would make commandStop/parseBreakpointSpec logic more clear:
>> commandStop handles go/thread/threadid, parseBreakpointSpec handles 
>> position (part starting from in/at);
>>
>> - do you think BreakpointSpec toString should contain info about the 
>> thread (i.e. to specified in the breakpoint list)?;
> I had added this at one point, alone with also adding the suspend 
> policy. You then see both of these when you use "stop in/at" or 
> "clear" without any arguments (it lists the breakpoints in this case). 
> I was a little worried that the extra text might break something, so I 
> left it out. Note also that currently the text given when you list 
> breakpoints is exactly the text you need to enter when using the clear 
> command, so that would no longer be true if I added any more text. 
> However, I did envision a better version of this that not only include 
> the suspend policy and threadid, but also a breakpoint index, allowing 
> you to more easily clear one after listing it. Maybe I could add an 
> RFE for this.
>>
>> - it would be nice to add a test for the new threadid functionality.
>
> I've been working on one today. It's near completion.
>
> Chris
>
>>
>> --alex
>>
>> On 02/19/2019 21:57, Chris Plummer wrote:
>>> Hello,
>>>
>>> Please review the following:
>>>
>>> http://cr.openjdk.java.net/~cjplummer/8219143/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8219143
>>>
>>> Normally when a breakpoint is set in jdb, it is set globally (all 
>>> threads). JDI supports the ability to have the breakpoint be 
>>> automatically filtered so it will only be delivered on a specified 
>>> thread and ignored on all other threads. This change allows making 
>>> use of that JDI feature from jdb. So instead of something like:
>>>
>>>    stop at Foo:23
>>>
>>> You can now do:
>>>
>>>    stop at threadid 7 Foo:23
>>>
>>> Where 7 is the threadID for the thread as seen in the output of the 
>>> "threads" command. The format of the stop command is now:
>>>
>>>     stop [go|thread] <at|in> [threadid <thread_id>] <location>
>>>
>>> It is still fully backwards compatible.
>>>
>>> As part of this change I also cleaned up the "stop" command parsing 
>>> and error handling. It was kind of a mess, and was near impossible 
>>> to add the "threadid" option until after I did much of the cleanup. 
>>> I've also cleaned up the help output a lot, and added help for the 
>>> "go" and "thread" options. One last change was to remove the 
>>> distinction between "stop at" and "stop in", which was suggested 
>>> already in a comment. They can be used interchangeably now. The 
>>> changes in Commands.java are pretty much all just the above cleanup, 
>>> except for the one short section with the 'Handle "threadid" 
>>> modifier' comment.
>>>
>>> thanks,
>>>
>>> Chris
>>>
>
>