RFR: 8210131: vmTestbase/nsk/jvmti/scenarios/allocation/AP10/ap10t001/TestDescription.java failed with ObjectFree: GetCurrentThreadCpuTimerInfo returned unexpected error code

[Kim Barrett]

> On Sep 3, 2018, at 3:12 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Kim,
> 
> On 31/08/2018 12:15 PM, Kim Barrett wrote:
>> Please review this small change to GetCurrentThreadCpuTimerInfo and
>> GetCurrentThreadCpuTime JVMTI operations, allowing them to be called
>> from any NamedThread.  This is consistent with the behavior from
>> JDK-8203948; there is no obvious reason for these operations to have
>> any thread context restriction.
>> These functions are documented as being callable from heap callbacks
>> like ObjectFree, which can be called from an "internal thread or the
>> thread that called the [heap] iteration function".  However, unlike
>> other, similar functions, these two were requiring the current thread
>> to be either a JavaThread or the VMThread.  For other such functions,
>> JDK-6278106 had added Concurrent GC threads, and JDK-8203948
>> generalized that to any NamedThread (or JavaThread).  This change
>> makes these functions behave similarly.
> 
> As previously discussed** we could just relax the thread constraint completely here. For most JVM TI callbacks the caller must be a JavaThread, but for callbacksafe cases it can nominally be any thread.
> 
> I don't like the fact NamedThread is getting used to imply a capability that is totally unrelated to being named.
> 
> **Though if you saw my initial comment on the bug you'll also realize I can't make up my mind whether to enumerate allowed threads or not. :)

NamedThread already has a certain amount of additional protocol, e.g.
the GC id and the processed thread (whose purpose I haven't explored).
Currently, all NamedThreads except VMThread are specifically GC
threads. I think the GC id is probably relevant to the problem at
hand, in that only a GC-cognizent thread is going to be making these
fairly GC-related callbacks. I won't argue that this is good factoring
or naming though.  As I said over in the RFR thread for 8209976, this
seems like an area that could use some cleanup.

I'm not sure where you are thinking that enumeration might be made?
Certainly not in the JVMTI spec; it already says "internal thread" and
I don't think it should say anything more than that.  Maybe you mean
in the code?  Presently, NamedThread occupies what seems to be the
appropriate place.

> Cheers,
> David
> 
>> This is needed to allow such callbacks to be invoked from a GC worker
>> thread, such as is now done for G1 (JDK-8072498), and probably other
>> collectors in the future.  (See comments for JDK-8203948 for a
>> discussion of why ZGC didn't run into this problem.)
>> This change also removes the failing test from the problem list.
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8210131
>> Webrev:
>> http://cr.openjdk.java.net/~kbarrett/8210131/open.00/
>> Testing:
>> mach5 tier1-3, hs-tier4-6.
>> Local (linux-x64) jtreg:hotspot_gc, and manually ran the failing test.
>> Examined the generated jvmtiEnter[Trace].cpp file and verified the
>> change to the generated code was as expected.