2-nd round RFR 6471769: Error: assert(_cur_stack_depth == count_frames(), "cur_stack_depth out of sync")

[Daniel D. Daugherty]

On 2/27/14 1:25 AM, 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-DEPTH.2 
>

src/share/vm/runtime/vm_operations.hpp
     No comments.

src/share/vm/prims/jvmtiEnvBase.hpp
     No comments.

src/share/vm/prims/jvmtiEnv.cpp
     No comments.

src/share/vm/prims/jvmtiEnvThreadState.cpp
     No comments.

src/share/vm/prims/jvmtiEventController.cpp
     JvmtiEventController::set_frame_pop() is called by
     JvmtiEnvThreadState::set_frame_pop() which is called by
     JvmtiEnv::NotifyFramePop().

     The "MutexLocker mu(JvmtiThreadState_lock)" in
     JvmtiEventController::set_frame_pop() protected the work
     done by JvmtiEventControllerPrivate::set_frame_pop():

       ets->get_frame_pops()->set(fpop);
recompute_thread_enabled(ets->get_thread()->jvmti_thread_state());

     Since multiple threads can call JVM/TI NotifyFramePop() on the
     same target thread, what keeps the threads from messing with
     the list of frame pops simultaneously or messing with the
     thread enabled events bits in parallel?

     I suspect that this might also be an issue for
     JvmtiEventController::clear_frame_pop() and
     JvmtiEventController::clear_to_frame_pop() also.

src/share/vm/prims/jvmtiThreadState.cpp
     No comments.

Dan


>
> Summary:
>
>   It is the 2-nd round of review because the JTREG com/sun/jdi tests 
> discovered a regression
>   in the first round change. The issue was in the 
> JvmtiEventController::clear_frame_pop()
>   lock synchronization that is not allowed at safepoints.
>
>   As a result I've changed the JvmtiEnv::NotifyFramePop to use a VM 
> operation for safety.
>   Also, I've removed the lock synchronization from the 3 impacted 
> JvmtiEventController::
>   functions: set_frame_pop(), clear_frame_pop() and clear_to_frame_pop().
>
> Testing:
>   In progress: nsk.jvmti, nsk.jdi, nsk.jdwp, JTreg com/sun/jdi
>
>
> Thanks,
> Serguei
>
>
> On 2/25/14 12:43 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-DEPTH.1 
>>
>>
>> Summary:
>>
>>   This is another Test Stabilization issue.
>>   The fix is very similar to other JVMTI stabilization fixes.
>>   It is to use safepoints for updating the PopFrame data instead of 
>> relying on the
>>   suspend equivalent condition mechanism 
>> (JvmtiEnv::is_thread_fully_suspended())
>>   which is not adequate from the reliability point of view.
>>
>> Testing:
>>   In progress: nsk.jvmti, nsk.jdi, nsk.jdwp, JTreg com/sun/jdi
>>
>>
>> Thanks,
>> Serguei
>>
>