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>
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)?;
- it would be nice to add a test for the new threadid functionality.
--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
