On 11/15/19 4:39 AM, [email protected] wrote:
On 11/14/19 9:07 PM, Chris Plummer wrote:
Hi Coleen,
Is it ok to end up missing some CompiledMethodLoad events? The spec
says:
"Sent when a method is compiled and loaded into memory by the VM. If
it is unloaded, the CompiledMethodUnload event is sent. If it is
moved, the CompiledMethodUnload event is sent, followed by a new
CompiledMethodLoad event. Note that a single method may have multiple
compiled forms, and that this event will be sent for each form. "
So a method was still "compiled and loaded into memory", right? We
just didn't get the event out before it was too late. Is the
CompiledMethodUnload still sent?
Yes, the CompiledMethodUnload event would be sent for this.
My first version of my change reported the event without the extra
information (inlining and some code blob address location maps). Maybe
that would be better. Here it is and tested successfully with the
testcase that crashed.
open webrev at http://cr.openjdk.java.net/~coleenp/2019/8173361.02/webrev
The more I look at this code, the more problems I see.
get_and_cache_jmethod_id() will send back a jmethodID from when the
method was live. But like the class unloading event, a user cannot
trust that the Method* in the jmethodID points to anything valid.
So the spec for CompiledMethodLoad event should say for the method,
like the CompiledMethodUnload event:
Compiled method being unloaded. For identification of the compiled
method only -- the class may be unloaded and therefore the method
should not be used as an argument to further JNI or JVMTI functions.
Yes, I agree that with this approach a spec clarification is needed. I
just wonder about compatibility for agents that assume it is a valid
jmethodID. I suppose if an agent treated it as valid and crashed as a
result, this is just moving the crash from the jvmti impl to the agent.
We also have to consider that agents might currently treat it as valid
for functional purposes, and likely never run into this problem, but
with the spec update technically that would mean that they would no
longer be able to. However, what's likely is that any existing agent
would just continue to be ignorant of this spec change, and continue to
run with no issue.
Or we don't send the event like 01. Either one doesn't crash.
Yeah, I guess I don't have a good answer here. Seems like both
approaches have issues. Maybe the correct fix is to keep the nm live
until the deferred event can be sent.
thanks,
Chris
Thanks,
Coleen
thanks,
Chris
On 11/14/19 5:15 PM, [email protected] wrote:
Summary: Don't post information which uses metadata from unloaded
nmethods
Tested tier1-3 and 100 times with test that failed (reproduced
failure without the fix).
open webrev at
http://cr.openjdk.java.net/~coleenp/2019/8173361.01/webrev
bug link https://bugs.openjdk.java.net/browse/JDK-8173361
Thanks,
Coleen
