[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(XS): 8158033: notify_tracing() misplaced for intended purpose
From:       David Holmes <david.holmes () oracle ! com>
Date:       2016-06-01 4:03:05
Message-ID: 3aaa6264-31de-debd-ec9a-facc31c9d10d () oracle ! com
[Download RAW message or body]

Thanks for the additional info. I guess we will see how this behaves as 
time progresses.

Reviewed.

David

On 28/05/2016 2:11 AM, Markus Gronlund wrote:
> 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
> > 


[prev in list] [next in list] [prev in thread] [next in thread]