FW: RFR(S): 8210024: JFR calls virtual is_Java_thread from ~Thread()

[Markus Gronlund]

Thanks David,

"is there any restriction/requirement/expectation that Jfr::on_thread_exit(t) is called from t? For JavaThreads it is always; for NonJavaThreads it may or may not."

Great question.

Presuppositions:

For JavaThreads:        Thread::current() == t  // only because they also issue thread local events (ThreadEndEvent, ThreadCPULoadEvent) as part of exit.
For NonJavaThreads: (Thread::current() == t || Thread::current() != t) // the buffer release code supports execution by a thread that is not the holder.

I will add an assert in the JavaThread specific code sections of Jfr::on_thread_exit(t) , that Thread::current() == t.

Thanks again for good input.
Markus


-----Original Message-----
From: David Holmes 
Sent: den 14 november 2018 10:05
To: Markus Gronlund <markus.gronlund at oracle.com>; hotspot-runtime-dev at openjdk.java.net
Subject: Re: FW: RFR(S): 8210024: JFR calls virtual is_Java_thread from ~Thread()

Hi Markus,

The changes to the runtime code in thread.cpp seems fine.

The Jfr changes I can't really speak to, but they seem reasonable. One
question: is there any restriction/requirement/expectation that
Jfr::on_thread_exit(t) is called from t? For JavaThreads it is always; for NonJavaThreads it may or may not.

Thanks,
David
-----

On 14/11/2018 6:50 pm, Markus Gronlund wrote:
> Greetings,
> 
> This RFR should have been sent to hotspot-runtime-dev as well as part of the original posting.
> 
> Thank you
> Markus
> 
> -----Original Message-----
> From: Markus Gronlund
> Sent: den 12 november 2018 13:59
> To: hotspot-jfr-dev at openjdk.java.net; Kim Barrett 
> <kim.barrett at oracle.com>
> Subject: RFR(S): 8210024: JFR calls virtual is_Java_thread from 
> ~Thread()
> 
> Greetings,
> 
> Please review the following changes to help make the JFR deregistration / buffer release steps a bit more robust as thread exits.
> 
> There is a need to release the JNIHandle for the EventWriter while the JavaThread is still on the threads list, since there is a sensitive interplay with the G1-prebarrier, and this change will help assist in the removal of shared PtrQueues.
> 
> The virtual is_Java_Thread() call can still be invoked for the NonJavaThreads in their destructor, but since the thread is still a NonJavaThread at this point, it should be possible to make is_Java_thread() pure virtual in Thread.
> I did not find any proper non-platform specific exit location for NonJavaThreads, so they release as part of the destructor.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8210024
> Webrev: http://cr.openjdk.java.net/~mgronlun/8210024/webrev01/
> Testing: JFR tests, tier1, tier2, tier3
> 
> Comments:
> The JfrThreadSampler changes should have been done when the JfrThreadSampler thread was turned into a NonJavaThread - removing the dead export code as part of this change.
> The Jfr::on_java_thread_dismantle() is an intermediary support that will be removed when we change overall thread registration from on_exit() to on_start(), which are planned upcoming changes.
> 
> Thanks to Kim for detailing several involved issues.
> 
> Thanks
> Markus
>