On Tue, Jun 19, 2018 at 4:23 AM, David Holmes <[email protected]> wrote: > Hi Fred, > > On 19/06/2018 6:52 AM, Frederic Parain wrote: >> >> Hi David, >> >> Thank you for looking at this issue. >> >> The intend has never been to provide a command to easily crash the JVM :-) XD
>> The patch was provided without any safety measures in purpose: to enable >> explorations in order to see how badly things could go south. >> >> The final design must include a per-command flag or method specifying if >> the command can be executed inside a safepoint. This information has to >> be provided by JVM developers, as there’s no automatic way to determine if >> a command is safepoint safe or not. >> >> So far, we already have the following rules to decide if a command can be >> executed at a safepoint or not: >> - it must not perform Java callbacks >> - it must not take locks >> - it must not invoke VMOp relying on the execution of their prologue >> and >> epilogue >> >> Let’s see: >> 1 - if there are more restrictions to consider >> 2 - what can be achieved with respect of these restrictions (which >> commands >> can run at a safepoint, which commands can easily be modified to >> comply >> with the restrictions above) > > > Doesn't each Dcmd already explicitly get implemented as either a VMOp or > not? I would not be concerned with trying to run non-VMop Dcmds at a > safepoint - what does that buy us? The issue as I understood it was the > problem of requiring a series of safepoints for a series of > safepoint-needing commands. Or are there useful sequences of interleaving > VMop and non-VMop actions that would benefit from this? It does not hurt either, no? Just from the usability standpoint I would allow bundling non-VMop commands with VMop commands. I only would explicitly forbid those commands which must not run at a safepoint. Note that one hope I have with such a command is that it will encourage small, clean command design unix style a la "do one thing and do it right" - right now, if you want to have a consistent picture involving multiple information containing oop, you'd have to write a single command doing it all together and have to run it at a safepoint. With Freds command, we can write small singular commands which output oops and if we need oop consistency, we bundle them in one safe point. Best Regards, Thomas > > Cheers, > David > > >> Regards, >> >> Fred >> >>> >>> On Jun 17, 2018, at 22:08, David Holmes <[email protected]> wrote: >>> >>> Hi Fred, >>> >>> Thanks for this and the analysis below. >>> >>> My 2c is that it is far too risky to provide a general purpose facility >>> like this where arbitrary command combinations can result in crashing the >>> target VM! >>> >>> Certainly no command that requires a JavaThread should be allowed to be >>> requested - only commands actually implemented via safepointing VMOps should >>> be considered. And then we should perhaps constrain things further so that >>> no commands that require prolog or epilog logic are accepted. >>> >>> Turning things around, perhaps we should flag which Dcmds, that operate >>> at a safepoint, are considered "combinable" and only allow those commands to >>> be specified? >>> >>> If we want a general purpose way of expressing a list of Dcmds to execute >>> then perhaps it should be specified to process each command in turn, with a >>> sequence of combinable safepoint ops processed at a single safepoint? >>> >>> Thanks, >>> David >>> >>> On 16/06/2018 1:32 AM, Frederic Parain wrote: >>>> >>>> Hi, >>>> Here’s a first prototype to explore running multiple commands in a >>>> single safepoint: >>>> http://cr.openjdk.java.net/~fparain/DCmdRunAtSafepoint/webrev.00/ >>>> The syntax is very simple, with the character ‘+’ used as the delimiter >>>> between commands. >>>> $ ./build/macosx-x64-debug/images/jdk/bin/jcmd 21164 VM.run_at_safepoint >>>> VM.version + VM.uptime >>>> 21164: >>>> Java HotSpot(TM) 64-Bit Server VM version >>>> 11-internal+0-2018-05-25-2202478.fparain.DCmd >>>> JDK 11.0.0 >>>> 44.143 s >>>> Playing a bit with it, it quickly shows what are the issues when trying >>>> to run >>>> diagnostic commands at a safepoint. >>>> Issue 1: One important point to remember is that only the VMThread can >>>> run at a safepoint, >>>> all JavaThreads are strictly forbidden to run during the safepoint. So >>>> executing commands at a >>>> safepoint means they will be executed by the VMThread. Unfortunately, >>>> the VMThread is >>>> subject to restrictions and cannot execute all code that a JavaThread >>>> can execute. This >>>> issue is visible with the VM.info command, which takes the Heap_lock. >>>> The VMThread >>>> is not allowed to grab this lock because it could cause the VMThread to >>>> be blocked by >>>> a JavaThread. So far, VM.info cannot be run at a safepoint. >>>> Issue 2: Executing a code at a safepoint requires a VMOperation. >>>> VMOperations are >>>> made of a prologue, a doit() method witch contains the code executed at >>>> the safepoint, >>>> and an epilogue. The prologue and the epilogue are executed by the >>>> requester of the >>>> VMOperation, usually a JavaThread, and they are not executed at a >>>> safepoint. Prologues >>>> and epilogues are designed to execute code the VMThread cannot execute, >>>> like taking >>>> locks or loading classes. When the VMThread itself wants to execute a >>>> VMOperation, >>>> what is called a nested VMOperation, it only executes the doit() method, >>>> skipping the >>>> prologue and the epilogue. This is an issue for diagnostic commands >>>> using a VMOp >>>> in their implementation: when these commands are called while the JVM is >>>> already >>>> at a safepoint, their prologue and epilogue cannot be executed. This >>>> issue impacts >>>> Thread.print and GC.run. >>>> There are probably other issues still to be found, so I encourage people >>>> to test >>>> this patch to see if the combinations of commands they would like to run >>>> at a >>>> safepoint can indeed be run in this mode. >>>> Note that the support for invoking the new command from the >>>> DiagnosticCommand >>>> MBean has not be implemented. This is a not negligible work in order to >>>> correctly >>>> check Java Permissions before executing the commands. >>>> Let me know if you consider this code useful or if the limitations >>>> described above >>>> make this feature not as pretty as we wanted it to be. >>>> Regards, >>>> Fred >>>>> >>>>> On May 29, 2018, at 15:54, Frederic Parain <[email protected]> >>>>> wrote: >>>>> >>>>> Hi Thomas, >>>>> >>>>> >>>>>> On May 26, 2018, at 04:07, Thomas Stüfe <[email protected]> >>>>>> wrote: >>>>>> >>>>> <snip> >>>>>>> >>>>>>> >>>>>>> I think that it is a fundamental piece of the design. Adding a new >>>>>>> diagnostic >>>>>>> command to group several commands under a unique safepoint: >>>>>>> 1 - requires explicit action from the user (and acknowledgment of the >>>>>>> risks) >>>>>>> 2 - doesn’t touch the current syntax/semantic at all >>>>>>> >>>>>>> Another reason I’m pushing for a specific diagnostic command rather >>>>>>> than >>>>>>> changing the syntax is remote execution. jcmd is not the only way to >>>>>>> invoke >>>>>>> a diagnostic command, they can also be invoke remotely using JMX and >>>>>>> the >>>>>>> com.sun.management.DiagnosticCommand dynamic MBean. Through the >>>>>>> platform MBean, users do not have access to the jcmd syntax, they see >>>>>>> each diagnostic command as a MBean operation (providing that they >>>>>>> have >>>>>>> been registered with the DCmd_Source_MBean flag). The initial support >>>>>>> for remote execution was taking a single String and processed it just >>>>>>> like >>>>>>> a string passed by jcmd, but people developing consoles for remote >>>>>>> management complained that this was not convenient for them, and >>>>>>> asked >>>>>>> for something they could use more programmatically. >>>>>>> If the safepoint bundling is implemented in the syntax, JMX clients >>>>>>> won’t >>>>>>> be able to use the feature. If we implement it with a new diagnostic >>>>>>> command, >>>>>>> it will be visible from the MBean and they will be able to use it. >>>>>> >>>>>> >>>>>> That is interesting. Do you have examples for such programs? I was >>>>>> looking for ways of how to use jcmd (or equivalent) remotely. >>>>> >>>>> >>>>> If you’re looking for a console application to invoke diagnostic >>>>> command >>>>> on a remote JVM, Mission Control has a dedicated support for that in >>>>> the >>>>> “Diagnostic Commands” tab. >>>>> If you’re looking for exemples of source code doing remote invocations, >>>>> there’re several unit tests for the DiagnosticCommand MBean in the >>>>> directory >>>>> test/jdk/com/sun/management/DiagnosticCommandMBean. >>>>> >>>>>> >>>>>>> >>>>>>> At last, creating a specific diagnostic command for safepoint >>>>>>> bundling would >>>>>>> show the way on how to group commands for consistency. Safepoints is >>>>>>> not >>>>>>> the only way to get consistency. For some JVM internals reasons, you >>>>>>> might >>>>>>> want to group commands under a single lock, like the Threads_lock. >>>>>>> Adding >>>>>>> a diagnostic command for each type of bundling seems more extensible. >>>>>>> >>>>>> >>>>>> You make good points. >>>>>> >>>>>> Adding a new command has also the advantage that it does not affect >>>>>> downward compatibility and hence would not require a CSR. CSR seems to >>>>>> slow down things a bit (I have a very simple CSR for jcmd open >>>>>> undecided since 12 days: >>>>>> https://bugs.openjdk.java.net/browse/JDK-8203043). So, while I think >>>>>> CSR are a necessary process, I am happy to avoid them if possible. >>>>>> >>>>>> Lets spin this out a bit more: >>>>>> >>>>>> -> However the user specifies "please execute these commands at a >>>>>> single safepoint", if one of the enclosed commands cannot be run >>>>>> inside a safepoint the whole bundle should fail, agreed? So, >>>>>> specifying e.g. JFR.start as part of such a command bundle would cause >>>>>> the whole bundle to not being executed. >>>>> >>>>> >>>>> Yes. The new command N would ask for the execution of a list of >>>>> commands { A, B, C, … }, and would fail if: >>>>> - any of the specified commands { A, B, C, … } is not a registered >>>>> command >>>>> - any of the specified commands { A, B, C, … } doesn’t support >>>>> execution at safepoint >>>>> - any of the specified commands { A, B, C, … } is not exported to >>>>> the DCmdSource used to invoke N (*) >>>>> >>>>> (*) Not all commands are exported to all interfaces (jcmd,JMX), for >>>>> instance, some commands related to the JMX agent are cannot >>>>> be invoked through the JMX. Even if the new command can be >>>>> invoked from all interfaces, it must not allow to circumvent this >>>>> restriction. This should not be an issue today, because all >>>>> JMX agent commands are calling back Java code, and are not >>>>> suitable for execution at a safepoint. But this would ensure the >>>>> consistency of the model in the long term (and the test is cheap). >>>>> >>>>>> >>>>>> -> If we introduce a command like you propose (lets call it "VM.run" >>>>>> for the sake of the argument) we still need to decide how to delimit >>>>>> the commands at the command line. That brings us back to the syntax >>>>>> question: separation by a explicit delimiter, if possible one that >>>>>> does not clash with common unix shells? >>>>>> >>>>>> Lets try colon ':' : >>>>>> >>>>>> jcmd VM.run VM.uptime : VM.metaspace show-loaders show-classes >>>>>> by-chunktype : VM.classloaders show-classes : VM.info >>>>>> >>>>>> Of course, we can wrap all arguments to jcmd in quotes, then the >>>>>> delimiter sign does not affect things as much, unless we use quotes :) >>>>>> We could e.g. use semicolon: >>>>>> >>>>>> jcmd ' VM.run VM.uptime ; VM.metaspace show-loaders show-classes >>>>>> by-chunktype ; VM.classloaders show-classes ; VM.info ‘ >>>>>> >>>>> >>>>> I thought about colon and semi-colon too, because they are the most >>>>> obvious delimiters. However, they are already as delimiters in >>>>> classpaths >>>>> (semi-colon on Unix, colon on Windows), so parsing to find the right >>>>> delimiters would be tricky. >>>>> >>>>> I’d start with ‘!’ or ‘+’ in a first time, to make some progress on the >>>>> other >>>>> aspects of the command, and buy some time to discuss the delimiter >>>>> issue in parallel. >>>>> >>>>> The last resort would be to force each command in the bundle to >>>>> be specified with quotes (arguments included). But it would make the >>>>> syntax very heavy, I’d prefer a simpler delimiter. >>>>> >>>>> >>>>>> Or, we go back to my original idea of letting the command keyword be >>>>>> the indicator for a new command start. Honestly, at this point I have >>>>>> no preference. Both approaches seem reasonable. >>>>>> >>>>>> The only niggle I have with an explicit command is the syntax, which I >>>>>> find a bit awkward. My thinking before reading your response was that >>>>>> I'd prefer a jcmd flag to specify the same behavior: that all commands >>>>>> should be executed at a single safepoint. E.g. [-s/--stacked] >>>>>> >>>>>> jcmd -s ' VM.uptime ; VM.metaspace show-loaders show-classes >>>>>> by-chunktype ; VM.classloaders show-classes ; VM.info ' >>>>>> >>>>>> But I see the disadvantage: depending on how it is implemented this >>>>>> would require consistent changes in both jcmd and hotspot, and not >>>>>> work with other clients using MBeans. Whereas the hypothetical >>>>>> "VM.run" would work out of the box for any old jcmd version and any >>>>>> other client. >>>>>> >>>>>> Of course one could keep the "VM.run" idea and still give jcmd a >>>>>> "-s/--stacked" option, as a shallow convenience feature, which just >>>>>> translates to "VM.run" and runs that. >>>>>> >>>>>> -> A variation of your "have special commands to run a command bundle >>>>>> in one particular context" theme could be having only one command, >>>>>> e.g. "VM.run", specifying the conditions under which to run the bundle >>>>>> as sub options. E.g. "VM.run -s <commands>" for "run commands at >>>>>> safepoint", "VM.run -t <commands>" for "run commands under >>>>>> Threads_lock". That would even allow to combine these contexts. I do >>>>>> not know if that makes sense. >>>>>> >>>>>> - Another advantage of having an explicit command just occurring to me >>>>>> is that it enables more evolved command files, where one can >>>>>> intersperse command bundles and single commands. For instance, I could >>>>>> give this to a customer: >>>>>> >>>>>> <command file> >>>>>> 1 VM.uptime >>>>>> 2 VM.run_commands VM.metaspace show-loaders show-classes by-chunktype >>>>>> VM.classloaders details VM.classloader_stats >>>>>> 3 GC.run >>>>>> 4 VM.run_commands VM.metaspace show-loaders show-classes by-chunktype >>>>>> VM.classloaders details VM.classloader_stats >>>>>> >>>>>> to execute a command bundle before and after triggering a GC. >>>>> >>>>> >>>>> This would work fine . The parser of the jcmd string will split the >>>>> string based on ‘\n’ characters, then each line of command will be >>>>> executed >>>>> sequentially, and lines containing the VM.run_command will then create >>>>> the >>>>> safepoint and trigger the execution of the different commands specified >>>>> in >>>>> its argument list. >>>>> >>>>>> >>>>>>> We have a three days week-end in the US, but I should be able to code >>>>>>> something next week. >>>>>>> >>>>>> >>>>>> Oh, you want to take over the implementation, sure! Fine by me. >>>>> >>>>> >>>>> I’m not promising an implementation, I haven’t worked on this code >>>>> since I left the serviceability team. I need to code at least a >>>>> skeleton >>>>> of the command to refresh my memory about all aspects of the >>>>> diagnostic command framework. >>>>> >>>>>> >>>>>>> Thank you for your ideas and your interest in diagnostic commands. >>>>>> >>>>>> >>>>>> Sure. Why I am interested in this: We strongly rely on tools similar >>>>>> to jcmd for our support. We have quite capable tools in our (licensed, >>>>>> non-open) VM port, which are similar to jcmd in many aspects. So we >>>>>> try to bring some of the capabilities upstream, in order to ease the >>>>>> merge costs for us and to help the OpenJDK community. >>>>> >>>>> >>>>> The diagnostic command framework was designed to support an >>>>> extensible set of commands. Feel free to extend, and thank you >>>>> for contributing to the OpenJDK community. >>>>> >>>>> Fred >>>>> >>>>>>> >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> A new separator, more convenient than newline, would be require to >>>>>>>>> have >>>>>>>>> a single line syntax. >>>>>>>>> >>>>>>>>> My 2 cents, >>>>>>>>> >>>>>>>> >>>>>>>> Certainly worth more than 2 cents :) Thanks for your thoughts! >>>>>>>> >>>>>>>> ..Thomas >>>>>>>> >>>>>>>>> Fred >>>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Erik >>>>>>>>>>> >>>>>>>>>>> Hi Thomas, >>>>>>>>>>> >>>>>>>>>>> very positive suggestion. >>>>>>>>>>> >>>>>>>>>>> One observation: >>>>>>>>>>> There already seems to be some different interpretation of the >>>>>>>>>>> command >>>>>>>>>>> line >>>>>>>>>>> in different Java versions: >>>>>>>>>>> For instance when I try to dump a flight recording in >>>>>>>>>>> 1.8.0_152-ea-b05 : >>>>>>>>>>> I >>>>>>>>>>> can use: >>>>>>>>>>> jcmd 33014 JFR.dump filename="recording1.jfr" recording=1 >>>>>>>>>>> but in build 9+181 , the same command must be: >>>>>>>>>>> jcmd 33014 JFR.dump filename="recording1.jfr",recording=1 >>>>>>>>>>> (the comma to separate sub-options you mentioned earlier) >>>>>>>>>>> >>>>>>>>>>> Therefore the suggestion without curly brackets, giving several >>>>>>>>>>> commands >>>>>>>>>>> after one another seems good with regard to backwards >>>>>>>>>>> compatibility. >>>>>>>>>>> >>>>>>>>>>> Motivation: hawt.io uses the MBean >>>>>>>>>>> com.sun.management:type=DiagnosticCommand >>>>>>>>>>> to access the same functionality as jcmd. Variations in option >>>>>>>>>>> syntax >>>>>>>>>>> across >>>>>>>>>>> Java versions is already an issue (only affecting sub-options >>>>>>>>>>> though, as >>>>>>>>>>> each command is a separate JMX operation). So syntax >>>>>>>>>>> compatibility is >>>>>>>>>>> highly >>>>>>>>>>> appreciated :-) >>>>>>>>>>> >>>>>>>>>>> Martin >>>>>>>>>>> >>>>>>>>>>> lør. 12. mai 2018 kl. 20:11 skrev Kirk Pepperdine >>>>>>>>>>> <[email protected]>: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> On May 10, 2018, at 11:26 AM, Thomas Stüfe >>>>>>>>>>>>> <[email protected]> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> On Thu, May 10, 2018 at 9:13 AM, Kirk Pepperdine >>>>>>>>>>>>> <[email protected]> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> The stacking at the safe point would be a huge win. Right now >>>>>>>>>>>>>> thread >>>>>>>>>>>>>> dump consistency can really confuse people as the tooling will >>>>>>>>>>>>>> show >>>>>>>>>>>>>> two >>>>>>>>>>>>>> threads owning the same lock at seemingly the same time. >>>>>>>>>>>>>> Reality, it’s >>>>>>>>>>>>>> just >>>>>>>>>>>>>> that people didn’t realize you get a safe point for each >>>>>>>>>>>>>> thread >>>>>>>>>>>>>> therefor >>>>>>>>>>>>>> there is motion in the system as you’re collecting the data. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> I am a bit confused. What tooling are you talking about? >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> jstack. I’ve not seen it with jcmd. But I often see 2 threads >>>>>>>>>>>> holding >>>>>>>>>>>> the >>>>>>>>>>>> same lock at the “same time” which is of course non-sense. I can >>>>>>>>>>>> dig one >>>>>>>>>>>> up >>>>>>>>>>>> for you if you’re still confused. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> As an aside, it’s amazing how many dev’s aren’t aware of jcmd. >>>>>>>>>>>>>> Just >>>>>>>>>>>>>> yesterday after my session at Devoxx I had someone ask me >>>>>>>>>>>>>> about using >>>>>>>>>>>>>> jfr >>>>>>>>>>>>>> from the command line, many in that session had not seen jcmd >>>>>>>>>>>>>> before. >>>>>>>>>>>>>> The >>>>>>>>>>>>>> feedback was, wow, this is very useful and I wished I had of >>>>>>>>>>>>>> known >>>>>>>>>>>>>> about it >>>>>>>>>>>>>> earlier. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Yes, jcmd is quite useful. I also really like the simple >>>>>>>>>>>>> design, which >>>>>>>>>>>>> is by default up- and downward compatible (so you can talk to >>>>>>>>>>>>> any VM, >>>>>>>>>>>>> new, old, it should not matter). That is just good design. We - >>>>>>>>>>>>> Sap - >>>>>>>>>>>>> work to extend jcmd, to help our support folks. E.g. the whole >>>>>>>>>>>>> new >>>>>>>>>>>>> VM.metaspace command chain. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> And simple it is….well done!!! >>>>>>>>>>>> >>>>>>>>>>>> — Kirk >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>> >>>>> >> >
