Re: RFR: 8318026: jcmd should provide access to low-level JVM debug information [v4]
On Mon, 4 Mar 2024 22:02:53 GMT, Kevin Walls wrote: >> Kevin Walls has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - Usage correction >> - Help to clarify this is VM inspection. Comment to relate source to >> debug.cpp. >> - jcheck trailing whitespace > > Hi Thomas @tstuefe - > > Security concerns certainly needed some thought. We have remote access to > DiagnosticCommands over JMX. I don't see a particular need for that for this > command. I think it was keeping the DCmd flagged as hidden that hides it > from the list of dcmds available in that way. So we have the attach api > controls using local user identity, as we do for everything else. > > Yes, "events" is not that useful, I should remove it. I was taking the > useful parts of debug.cpp, and although the events were in VM.info, I had > missed that since your JDK-8224600, VM.events is its own command now! > > Thread.print with stacks, vs "VM.debug threads" which is just a thread list, > so you have e.g. JavaThread*, name, native id, stack range. > > "find": Usage at the moment, would likely be MessageBoxOnError, and a native > debugger to get the native stacktrace and parameters that include an object > reference you care about. It might be other jcmds if e.g. events capture a > relevant problematic pointer, but likely it involves a native debugger. > Using jcmd behaves better, showing the output where you run jcmd, not in the > VM's current output as the debug.cpp utils do. > Native debugger syntax to call abritrary functions can be awkward, > particularly on Windows, so the jcmd should be a better experience. > > This might be more compelling in post-mortem usage. Am working on that. > i.e. jcmd on a core file. But I am saying it offers some value today. > > The class/method finders, I've heard some enthusiasm for their inclusion. We > don't want to encourage overlap but yes we do have some overlapping jcmds. > > > This is an umbrella but I don't think it's vague. VMDebugDcmd is for > inspecting VM state. It's inspired by debug.cpp utilities, does not need to > implement all of them, but does aim to make them more accessible (I will > assert that they are not well known, which is hard to prove.) > > Do we have a problem with jcmd feature creep? If anything has crept too far > it can be addressed. Looks like the DCmd classes have approximately doubled > since jdk8u but this looks like growth in a good way. > > Thanks > Kevin Hi Kevin, @kevinjwalls , thank you for your explanations. Please find answers inline. > Security concerns certainly needed some thought. We have remote access to > DiagnosticCommands over JMX. I don't see a particular need for that for this > command. I think it was keeping the DCmd flagged as hidden that hides it from > the list of dcmds available in that way. So we have the attach api controls > using local user identity, as we do for everything else. > > Yes, "events" is not that useful, I should remove it. I was taking the useful > parts of debug.cpp, and although the events were in VM.info, I had missed > that since your JDK-8224600, VM.events is its own command now! > > Thread.print with stacks, vs "VM.debug threads" which is just a thread list, > so you have e.g. JavaThread*, name, native id, stack range. > > "find": Usage at the moment, would likely be MessageBoxOnError, and a native > debugger to get the native stacktrace and parameters that include an object > reference you care about. It might be other jcmds if e.g. events capture a > relevant problematic pointer, but likely it involves a native debugger. Using > jcmd behaves better, showing the output where you run jcmd, not in the VM's > current output as the debug.cpp utils do. Native debugger syntax to call > abritrary functions can be awkward, particularly on Windows, so the jcmd > should be a better experience. I remain sceptic here, because in my experience, once you start poking at the JVM innards at this level, I guess you will be quickly at the limit of what this command can do for you and need to attach a debugger anyway. Could this be an own command, e,g, `VM.inspect`, and possibly limited to debug VMs? Do we really need this feature in production? > > This might be more compelling in post-mortem usage. Am working on that. i.e. > jcmd on a core file. But I am saying it offers some value today. Don't we have jhsdb for that? > > The class/method finders, I've heard some enthusiasm for their inclusion. We > don't want to encourage overlap but yes we do have some overlapping jcmds. Yeah, I can see this being useful. > > This is an umbrella but I don't think it's vague. VMDebugDcmd is for > inspecting VM state. It's inspired by debug.cpp utilities, does not need to > implement all of them, but does aim to make them more accessible (I will > assert that they are not well known, which is hard to prove.) > > Do we have
Re: RFR: 8318026: jcmd should provide access to low-level JVM debug information [v4]
On Mon, 4 Mar 2024 15:10:12 GMT, Kevin Walls wrote: >> Introduce the jcmd "VM.debug" to implement access to a useful set of the >> established debug.cpp utilities, with "jcmd PID VM.debug subcommand ...". >> >> Not recommended for live production use. Calling these "debug" utilities, >> and not including them in the jcmd help output, is to remind us they are not >> general customer-facing tools. > > Kevin Walls has updated the pull request incrementally with three additional > commits since the last revision: > > - Usage correction > - Help to clarify this is VM inspection. Comment to relate source to > debug.cpp. > - jcheck trailing whitespace Hi Thomas @tstuefe - Security concerns certainly needed some thought. We have remote access to DiagnosticCommands over JMX. I don't see a particular need for that for this command. I think it was keeping the DCmd flagged as hidden that hides it from the list of dcmds available in that way. So we have the attach api controls using local user identity, as we do for everything else. Yes, "events" is not that useful, I should remove it. I was taking the useful parts of debug.cpp, and although the events were in VM.info, I had missed that since your JDK-8224600, VM.events is its own command now! Thread.print with stacks, vs "VM.debug threads" which is just a thread list, so you have e.g. JavaThread*, name, native id, stack range. "find": Usage at the moment, would likely be MessageBoxOnError, and a native debugger to get the native stacktrace and parameters that include an object reference you care about. It might be other jcmds if e.g. events capture a relevant problematic pointer, but likely it involves a native debugger. Using jcmd behaves better, showing the output where you run jcmd, not in the VM's current output as the debug.cpp utils do. Native debugger syntax to call abritrary functions can be awkward, particularly on Windows, so the jcmd should be a better experience. This might be more compelling in post-mortem usage. Am working on that. i.e. jcmd on a core file. But I am saying it offers some value today. The class/method finders, I've heard some enthusiasm for their inclusion. We don't want to encourage overlap but yes we do have some overlapping jcmds. This is an umbrella but I don't think it's vague. VMDebugDcmd is for inspecting VM state. It's inspired by debug.cpp utilities, does not need to implement all of them, but does aim to make them more accessible (I will assert that they are not well known, which is hard to prove.) Do we have a problem with jcmd feature creep? If anything has crept too far it can be addressed. Looks like the DCmd classes have approximately doubled since jdk8u but this looks like growth in a good way. Thanks Kevin - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-1977543469
Re: RFR: 8318026: jcmd should provide access to low-level JVM debug information [v4]
On Mon, 4 Mar 2024 15:10:12 GMT, Kevin Walls wrote: >> Introduce the jcmd "VM.debug" to implement access to a useful set of the >> established debug.cpp utilities, with "jcmd PID VM.debug subcommand ...". >> >> Not recommended for live production use. Calling these "debug" utilities, >> and not including them in the jcmd help output, is to remind us they are not >> general customer-facing tools. > > Kevin Walls has updated the pull request incrementally with three additional > commits since the last revision: > > - Usage correction > - Help to clarify this is VM inspection. Comment to relate source to > debug.cpp. > - jcheck trailing whitespace Of the sub commands given in the CSR: eventsHotSpot JVM Event log threads Thread list find ADDRESS Describe an address given by the argument findclass CLASS_PATTERN FLAGS findmethod CLASS_PATTERN METHOD_PATTERN FLAGS 1) `events` is redundant (we have VM.events, and events are printed as part of VM.info) 2) `threads` is handled by Thread.info 3) I am still puzzled about `find`. This is to ask a running VM about an arbitrary address. But how do you get such an address? Usually only via debugging. Then I am already attached with a system debugger. Under which circumstances would this be needed? Can you give a usage example? 4) and 5) shared some overlap with VM.classes, VM.class_hierarchy, VM.metaspace and various Compiler.xx commands. I am mostly worried about adding such a low-level debug command to jcmd. Especially in such a vague umbrella form. Experiences show that these commands get extended easily, often without CSR. So its an open door for feature creep. I mostly worry about unforeseen security consequences, especially if jcmd reach is extended across machine boundaries. Weighted against that, the benefits seem a bit slim to me. Again, real-world use cases would help. - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-1976941359
Re: RFR: 8318026: jcmd should provide access to low-level JVM debug information [v4]
> Introduce the jcmd "VM.debug" to implement access to a useful set of the > established debug.cpp utilities, with "jcmd PID VM.debug subcommand ...". > > Not recommended for live production use. Calling these "debug" utilities, > and not including them in the jcmd help output, is to remind us they are not > general customer-facing tools. Kevin Walls has updated the pull request incrementally with three additional commits since the last revision: - Usage correction - Help to clarify this is VM inspection. Comment to relate source to debug.cpp. - jcheck trailing whitespace - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/cacbca27..1d1e20e2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=02-03 Stats: 7 lines in 3 files changed: 1 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: 8318026: jcmd should provide access to low-level JVM debug information [v4]
On Tue, 27 Feb 2024 01:30:27 GMT, Serguei Spitsyn wrote:
>> Kevin Walls has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - Usage correction
>> - Help to clarify this is VM inspection. Comment to relate source to
>> debug.cpp.
>> - jcheck trailing whitespace
>
> src/hotspot/share/services/diagnosticCommand.cpp line 1200:
>
>> 1198: void VMDebugDCmd::find() {
>> 1199: if (!_arg1.has_value()) {
>> 1200: output()->print_cr("missing argument");
>
> I'm thinking if it would be useful to tell what arguments are expected? This
> is for all cases where the `"missing argument"` message is returned.
Thanks Serguei, have filled in explicit usage instructions when the commands
are missing an argument.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1511313428
