RFR (XS) 6471769: Error: assert(_cur_stack_depth == count_frames(), "cur_stack_depth out of sync")

[serguei.spitsyn at oracle.com]

On 2/3/14 2:38 PM, Daniel D. Daugherty wrote:
>
> On 2/3/14 12:12 PM, serguei.spitsyn at oracle.com wrote:
>> On 2/3/14 4:16 AM, David Holmes wrote:
>>> Hi Serguei,
>>>
>>> On 1/02/2014 12:58 PM, serguei.spitsyn at oracle.com wrote:
>>>> Please, review the fix for:
>>>>    https://bugs.openjdk.java.net/browse/JDK-6471769
>>>>
>>>>
>>>> Open webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/6471769-JVMTI-FRAME/ 
>>>>
>>>>
>>>>
>>>> Summary:
>>>>
>>>>    There is a general issue in the suspend equivalent condition 
>>>> mechanism:
>>>>    Two subsequent calls to the 
>>>> JvmtiEnv::is_thread_fully_suspended() may
>>>> return different results:
>>>>      - 1-st: true
>>>>      - 2-nd: false
>>>>
>>>>    This more generic suspend equivalent issue is covered by another 
>>>> bug:
>>>>      https://bugs.openjdk.java.net/browse/JDK-6280037
>>>>
>>>>    The bug to fix in this review is a specific manifestation of the 
>>>> 6280037
>>>>    in the JVMTI GetFrameCount() that has a big impact on the SQE 
>>>> nightly.
>>>>    It is on the Test Stabilization radar (as well as the 6280037).
>>>>    There are many tests intermittently failing because of this.
>>>>
>>>>    The webrev for review is a one-liner work around the 6280037 for 
>>>> the
>>>> GetFrameCount().
>>>>
>>>>    The JVMTI GetFrameCount() spec tells:
>>>>      "If this function is called for a thread actively executing
>>>> bytecodes (for example,
>>>>       not the current thread and not suspended), the information
>>>> returned is transient."
>>>>
>>>>    So, it is Ok to call the GetFrameCount() for non-suspended target
>>>> threads.
>>>>    To achieve safety, the frame count for non-suspended threads is
>>>> calculated at a safepoint.
>>>>    It should be Ok and more safe to do the same for suspended 
>>>> threads as
>>>> well.
>>>>    There is no big performance impact because it is already on a 
>>>> slow path.
>>>>    It is still important to avoid safepointing when the target 
>>>> thread is
>>>> current.
>>>
>>> This sounds completely reasonable - Reviewed.
>>>
>>> I can't check at the moment whether VM_GetFrameCount properly checks 
>>> for a live target thread?
>>
>> David, thank you for the review!
>> It looks like the check is missed.
>> Please, give me some time to make sure what is needed here.
>
> Check out the work done via this changeset:
>
> http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/c1d7040a1183
>
> Sergey did the push via:
>
>     JDK-8022836 JVM crashes in 
> JVMTIENVBASE::GET_CURRENT_CONTENDED_MONITOR
>                 and GET_OWNED_MONITOR
>     https://bugs.openjdk.java.net/browse/JDK-8022836
>
> but the real commentary about the bug is in this one:
>
>     JDK-7154963 crash in JvmtiEnvBase::get_current_contended_monitor()
>     https://bugs.openjdk.java.net/browse/JDK-7154963
>
> I think above code is what you want...


I know what to check, just need to make sure new check is in a right 
place. :)
One of the examples is:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8030027-JVMTI-HS101.1/

Thanks,
Serguei




>
> Dan
>
>
>>
>> Thanks!
>> Serguei
>>
>>>
>>> David
>>>
>>>>    The bug 6280037 should go out of the Test Stabilization radar 
>>>> (remove
>>>> the svc-nightly label)
>>>>    as the most of the impacted tests are covered by the 6471769.
>>>>
>>>>
>>>> Testing:
>>>>    In progress: nsk.jvmti, nsk.jdi, nsk.jdwp, impacted JTreg tests
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>
>