JDK-8219143: jdb should support breakpoint thread filters
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Fri Feb 22 23:13:23 UTC 2019
Hi Chris,
It looks good.
Thank you for the update!
Thanks,
Serguei
On 2/22/19 12:24 PM, Chris Plummer wrote:
> Please review the updated webrev:
>
> http://cr.openjdk.java.net/~cjplummer/8219143/webrev.02/index.html
>
> They syntax is now:
>
> stop [go|thread] [<thread_id>] <at|in> <location>
>
> Basically I just removed the need to specify "threadid" before
> <thread_id>.
>
> Testing is in progress.
>
> thanks,
>
> Chris
>
> On 2/21/19 6:56 PM, Chris Plummer wrote:
>> On 2/21/19 5:02 PM, serguei.spitsyn at oracle.com wrote:
>>> Chris,
>>>
>>> New spec for stop command is:
>>> "Usage: stop [go|thread] [threadid <thread_id>] <at|in> <location>\n"
>>> ...
>>> 253 " If \"thread\" is specified, only suspend the thread we stop
>>> in\n" +
>>> ...
>>> 255 " If [threadid <thread_id>] is specified, only stop in the
>>> specified thread" +
>>>
>>>
>>> I wonder if it can be changed to something like this:
>>> "Usage: stop [go|thread [<thread_id>]] <at|in> <location>\n"
>> That's not correct since it requires that you specify "go" or
>> "thread" if you want to specify the thread to stop in. Right now if
>> you do:
>>
>> stop threadid 5 Main:3
>>
>> It will only stop in threadid 5, but all threads will be suspended.
>> Your syntax does not support this. I could attempt to go with:
>>
>> "Usage: stop [go|thread] [<thread_id>] <at|in> <location>\n"
>>
>> Basically the same as before, but not require the use of "threadid".
>> I don't think it would be too hard. After dealing with "go" or
>> "thread", if the next token is not "at" or "in", then require that it
>> be a threadid integer.
>>
>> Chris
>>> ...
>>> 253 " If \"thread\" is specified, only suspend the thread we stop
>>> in\n" +
>>> ...
>>> 255 " If [<thread_id>] is specified, only stop in the specified
>>> thread" +
>>>
>>>
>>> What do you think?
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 2/21/19 16:46, serguei.spitsyn at oracle.com wrote:
>>>> On 2/21/19 09:19, Chris Plummer wrote:
>>>>> On 2/21/19 2:04 AM, serguei.spitsyn at oracle.com wrote:
>>>>>> Hi Chris,
>>>>>>
>>>>>> Not a full review yet.
>>>>>>
>>>>>> Just a couple of quick mismatches.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~cjplummer/8219143/webrev.01/src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTYResources.java.frames.html
>>>>>>
>>>>>> Is it okay that the two fragments below describe the same
>>>>>> differently? Do I miss anything.
>>>>> The first is the help output when there is an error parsing the
>>>>> "stop" command. The second is the help output when you type
>>>>> "help", which includes help for all commands.
>>>>
>>>> My question is that a couple of statements are missed in
>>>> theprintstopcommandusage.
>>>> The help output prints these two lines which are not present in the
>>>> printstopcommandusage:
>>>> 378 " -- set a breakpoint\n" +
>>>> 379 " -- if no options are given, the current list of breakpoints
>>>> is printed\n" +
>>>> Is it intentional?
>>>>
>>>>
>>>>>> 250 {"printstopcommandusage",
>>>>>> 251 "Usage: stop [go|thread] [threadid <thread_id>] <at|in>
>>>>>> <location>\n" +
>>>>>> 252 " If \"go\" is specified, immediately resume after stopping\n" +
>>>>>> 253 " If \"thread\" is specified, only suspend the thread we stop
>>>>>> in\n" +
>>>>>> 254 " If neither \"go\" nor \"thread\" are specified, suspend all
>>>>>> threads\n" +
>>>>>> 255 " If [threadid <thread_id>] is specified, only stop in the
>>>>>> specified thread" +
>>>>>> 256 " \"at\" and \"in\" have the same meaning\n" +
>>>>>> 257 " <location> can either be a line number or a method:\n" +
>>>>>> 258 " <class_id>:<line_number>\n" +
>>>>>> 259 " <class_id>.<method>[(argument_type,...)]"
>>>>>> 260 },
>>>>>> 377 "stop [go|thread] [threadid <thread_id>] <at|in> <location>\n" +
>>>>>> 378 " -- set a breakpoint\n" +
>>>>>> 379 " -- if no options are given, the current list of breakpoints
>>>>>> is printed\n" +
>>>>>> 380 " -- if \"go\" is specified, immediately resume after
>>>>>> stopping\n" +
>>>>>> 381 " -- if \"thread\" is specified, only suspend the thread we
>>>>>> stop in\n" +
>>>>>> 382 " -- if neither \"go\" nor \"thread\" are specified, suspend
>>>>>> all threads\n" +
>>>>>> 383 " -- if [threadid <thread_id>] is specified, only stop in the
>>>>>> specified thread\n" +
>>>>>> 384 " -- \"at\" and \"in\" have the same meaning\n" +
>>>>>> 385 " -- <location> can either be a line number or a method:\n" +
>>>>>> 386 " -- <class_id>:<line_number>\n" +
>>>>>> 387 " -- <class_id>.<method>[(argument_type,...)]\n" +
>>>>>>
>>>>>>
>>>>>> http://cr.openjdk.java.net/~cjplummer/8219143/webrev.01/src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/Commands.java.frames.html
>>>>>>
>>>>>> 1162 * Allowed syntax:
>>>>>> 1163 * stop [go|thread] <at|in> [threadid <thread_id>] <location>
>>>>>> 1164 * <location> can either be a line number or a method:
>>>>>> 1165 * - <class id>:<line>
>>>>>> 1166 * - <class id>.<method>[(argument_type,...)]
>>>>>> 1167 * If "go" is specified, then immediately resume after
>>>>>> stopping. No threads are suspended.
>>>>>> 1168 * If "thread" is specified, then only suspend the thread we
>>>>>> stop in.
>>>>>> 1169 * If neither "go" nor "thread" are specified, then suspend
>>>>>> all threads.
>>>>>> 1170 * If the optional [threadid <thread_id>] is specified, then
>>>>>> only stop in the specified thread.
>>>>>> 1171 * If no options are given, the current list of breakpoints
>>>>>> is printed,
>>>>>> 1172 */The above is not aligned with the implementation as <at|in> has to be right before <location>.
>>>>>> A dot is needed instead of the last comma.
>>>>>> Should this comment to match one of the above descriptions?
>>>>> Yes, I'll update this to look like the help output. I missed it
>>>>> when moving the at/in location.
>>>>
>>>> Okay, thanks!
>>>>
>>>> I'll send full review soon.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>>
>>>>> thanks,
>>>>>
>>>>> Chris
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>> On 2/20/19 19:56, Chris Plummer wrote:
>>>>>>> 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
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190222/433e958c/attachment-0001.html>
More information about the serviceability-dev
mailing list