Hi Ralf,

Overall looks good, but I do have a few suggestions and questions:

I think I prefer passing EI_VM_INIT for triggering_ei, but if you prefer to keep it as is, I suggest a comment to clarify why it might be out of range.

The help for "oncmd" says "debug triggered by jcmd?". I think your use of "cmd" should be more consistent. It's a dcmd providing the service, and jcmd is just one way to get at the dcmd. I'm not sure of the best way to convey this simply in the option name and help, but it should be consistent. "ondcmd" is probably most accurate, but "onjcmd" is probably what is most meaningful to users. I don't think it makes sense to use "oncmd" if the help is referring to jcmd.

Your error handling in debugInit_startDebuggingViaCommand() is a bit inconsistent. For !allowStartViaCmd you immediately return the error. For !allowStartViaCmd you set the error, and then need to do error checks later before eventually returning the error. It think you could just return the error right away and remove the error checking code that comes later.

It's not clear to me why you want the name and address of the first transport. Why is the first one necessarily the one you want?

1403         bagEnumerateOver(transports, getFirstTransport, &spec);
1404
1405         if ((spec != NULL) && (transport_name != NULL) && (address != NULL)) {
1406             *transport_name = spec->name;
1407             *address = spec->address;
1408         }

thanks,

Chris

On 12/5/18 6:56 AM, Schmelter, Ralf wrote:
Hi All,

Please review the fix for the bug 
https://bugs.openjdk.java.net/browse/JDK-8214892. The webref is at 
http://cr.openjdk.java.net/~rrich/webrevs/schmelter/8214892/webrev.01/ .

This change allows debugging to be started delayed, triggered by a jcmd name 
VM.start_java_debugging. It works more or less like the onthrow and onuncaught 
options, but instead of triggering by exception we trigger it via the jcmd. 
This is useful when we want to trigger debugging when we see errors not related 
to exceptions (e.g. if a server becomes unresponsive it could be taken out of 
the routing and debugging is triggered to find the cause).

Best regards,
Ralf



Reply via email to