Re: 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.spit...@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.spit...@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