On 2/25/14 2:57 PM, Daniel D. Daugherty wrote:
On 2/25/14 1: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

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/jvmtiThreadState.cpp
    line 66:   _cur_stack_depth = UNKNOWN_STACK_DEPTH;
        This looks like the key piece of this fix with respect to the
        assert() in the bug report. I suspect that the first call to
        JvmtiThreadState::cur_stack_depth() is racing with another
        thread that happens to do something else that inits or sets
        _cur_stack_depth to an acceptable value.

Another potential cause of the issue is that the cur_stack_depth() is called from the update_for_pop_top_frame() which used to be called under suspend equivalent condition. The update_for_pop_top_frame() has been changed to be called at a safepoint now.


    line 251:     "must be current thread or at safepont");
    line 284:     "must be current thread or at safepont");
        typo: 'safepont' -> 'safepoint'

Thumbs up! No need to re-review the typo fixes.

Sure. I'll fix it before pushing.

Thanks, Dan!
Serguei


Dan



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



Reply via email to