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

[serguei.spitsyn at oracle.com]

Hi Kim,

The fix looks good to me too.
Thank you for taking care about this!

One comment below...

On 8/31/18 07:19, Daniel D. Daugherty wrote:
> On 8/30/18 10: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.
>>
>> 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/
>
> src/hotspot/share/prims/jvmtiEnter.xsl
>     No comments.
>
> test/hotspot/jtreg/ProblemList.txt
>     No comments.
>
>
> I verified that the XSL changes will only affect the two functions
> in question: jvmti_GetCurrentThreadCpuTimerInfo() and
> jvmti_GetCurrentThreadCpuTime(). Unfortunately, I can't remember
> why that bit of XSL code only applies to those two functions.

I was puzzled too but seems to understand it now.

The .xsl code limits these function to be defined with the attribute 
callbacksafe="safe"
and also (no 'phase' attribute or phase="live" or phase="start"):

<xsl:template match="function" mode="transition">
   . . .
   <xsl:choose>
     <xsl:when test="count(@callbacksafe)=0 or 
not(contains(@callbacksafe,'safe'))">
       <xsl:text>if (this_thread == NULL || 
!this_thread->is_Java_thread()) {</xsl:text>
     </xsl:when>
     <xsl:otherwise>
       <xsl:choose>
         <xsl:when test="count(@phase)=0 or contains(@phase,'live') or 
contains(@phase,'start')">
           <xsl:text>if (this_thread == NULL || 
(!this_thread->is_Java_thread() && 
!this_thread->is_VM_thread())) {</xsl:text>
         </xsl:when>
         <xsl:otherwise>
           <xsl:text>if (!this_thread->is_Java_thread()) {</xsl:text>
         </xsl:otherwise>
       </xsl:choose>
      </xsl:otherwise>
   </xsl:choose>
   . . .

This is the list of functions defined in the jvmti.xml with the 
attribute callbacksafe="safe":
% grep -r callbacksafe jvmti.xml | grep function
     <function id="Allocate" jkernel="yes" phase="any" 
callbacksafe="safe" impl="notrace" num="46">
     <function id="Deallocate" jkernel="yes" phase="any" 
callbacksafe="safe" impl="notrace" num="47">
     <function id="CreateRawMonitor" phase="onload" callbacksafe="safe" 
num="31">
     <function id="DestroyRawMonitor" phase="onload" callbacksafe="safe" 
num="32">
     <function id="RawMonitorEnter" phase="any" callbacksafe="safe" 
impl="innative notrace" num="33">
     <function id="RawMonitorExit" phase="any" callbacksafe="safe" 
impl="innative notrace" num="34">
     <function id="RawMonitorWait" phase="any" callbacksafe="safe" 
impl="innative notrace" num="35">
     <function id="RawMonitorNotify" phase="any" callbacksafe="safe" 
impl="notrace" num="36">
     <function id="RawMonitorNotifyAll" phase="any" callbacksafe="safe" 
impl="notrace" num="37">
     <function id="GetCurrentThreadCpuTimerInfo" callbacksafe="safe"  
impl="innative notrace" phase="start" num="134">
     <function id="GetCurrentThreadCpuTime" callbacksafe="safe" 
impl="innative notrace" phase="start" num="135">
     <function id="GetTimerInfo" phase="any" callbacksafe="safe" num="138">
     <function id="GetTime" phase="any" callbacksafe="safe" num="139">
     <function id="SetEnvironmentLocalStorage" jkernel="yes" phase="any" 
callbacksafe="safe" impl="innative notrace" num="148">
     <function id="GetEnvironmentLocalStorage" jkernel="yes" phase="any" 
callbacksafe="safe" impl="innative notrace" num="147">

Only the two of the functions above, GetCurrentThreadCpuTimerInfo and 
GetCurrentThreadCpuTime, have the attribute phase="start".
There are no functions in that list with the condition: no phase 
attribute or phase="live".


Thanks,
Serguei

>
> In any case, thumbs up!
>
> Dan
>
>
>>
>> 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.
>>
>