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

[David Holmes]

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
>