Hi David, Thanks for taking a look, pls see below.
Thanks again Markus -----Original Message----- From: David Holmes Sent: den 27 maj 2016 13:52 To: Markus Gronlund; serviceability-dev@openjdk.java.net Subject: Re: RFR(XS): 8158033: notify_tracing() misplaced for intended purpose Hi Markus, On 27/05/2016 7:33 PM, Markus Gronlund wrote: > Greetings, > > Please review this small fix: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8158033 > > Webrev: http://cr.openjdk.java.net/~mgronlun/8158033/webrev/ > > Description: > > The intent when putting in the notify_tracing() hook into debug.cpp > (report_java_out_of_memory()) was to intercept a state believed to be > a VM termination state, especially when OOME is thrown. Since it is > totally valid that Java code catches OOME, and this location actually > goes back to Java, this is the wrong location for this hook. > > In addition, the hook should not be typed for OOME only, but generic > for any exit condition (normal / OOME / crash). > This should instead have been put into java.cpp (before_exit()) and in > VMError.cpp (report_vm_die()). In src/share/vm/runtime/java.cpp why did you move the existing event code? What determined that TRACE_VM_EXIT should happen at that particular point? [MG] The reason I put in the TRACE_VM_EXIT (and moved up the event with it) here is because I would like the call to happen before the task that stops the WatcherThread. This is in order to have the "is_error_reported()" functionality still in place when calling TRACE_VM_EXIT. As this will be called when the VM is an unknown / corrupted state (crashing), I would like to have the timeout abort mechanism in place should TRACE_VM_EXIT run into anything that does not return properly (hangs). As for moving the event site, if TRACE_VM_EXIT deconstructs the tracing framework, it makes sense to have the event generated before this point. Aside, today, the current event site is located "too late" to be of real value. I also wonder what TRACE_VM_ERROR might do because in the vmError code it is called in a signal-handling context and so is very limited in what it can legitimately do without potentially messing up the error reporting. [MG] Yes this is sensitive I agree. I have put TRACE_VM_ERROR at a place where it will be able to handle reentrancy into the signal handler correctly. This is also tested by having the TRACE_VM_EXIT logic crash. In addition, TRACE_VM_ERROR will be made non-reentrant on initial call. Thanks again Markus Thanks, David > > > Thanks > > Markus >
