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

[Kim Barrett]

> On Nov 16, 2018, at 1:41 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
> 
>> On Nov 14, 2018, at 7:30 AM, Markus Gronlund <markus.gronlund at oracle.com> wrote:
>> 
>> Hi again,
>> 
>> Here is an updated webrev reflecting David's feedback.
>> 
>> This feedback actually discovered a bug in the release code, for the situation where a thread running the release for another threads thread local buffer (a situation that could manifest with NonJavaThreads), would have asserted deeper in the release code, because it would have attempted to reserve a promotion buffer using "t" instead of itself. Subsequent asserts would then have trapped in the check for Thread::current() == t. 
>> Thanks again David.
>> 
>> The updated webrev takes care of this situation as well as making the presuppositions (stated below) more explicit using assertions.
>> Webrev02: http://cr.openjdk.java.net/~mgronlun/8210024/webrev02/
> 
> I found it very confusing to be passing around a JfrThreadLocal / Thread pair that
> are unrelated.  Are they really allowed to be so?  It looks like it, but that’s pretty hard
> to determine.  Maybe the parameters should have more descriptive names, like
> “current_thread” or something (rather than “t” or “thread”), and I’m not sure what for
> the JfrThreadLocal to indicate it need not be related to the thread.  Or maybe the
> current thread shouldn’t be passed through all these layers, but should just be
> obtained down at the leaf where it’s needed?

Looks like the change has already been pushed, so perhaps take that as a
suggestion there might be further work to do in this area.