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
