Re: 2-nd round RFR (S) 8032223: nsk/regression/b4663146 gets assert(SafepointSynchronize::is_at_safepoint() || JvmtiEnv::is_thread_fully_suspended(get_thread(), false, &debug_bits))

[serguei.spit...@oracle.com]

On 2/4/14 12:45 PM, serguei.spit...@oracle.com wrote:

Thanks, Dan!
Some comments below.

On 2/4/14 7:48 AM, Daniel D. Daugherty wrote:

On 2/4/14 4:13 AM, serguei.spit...@oracle.com wrote:

Please, review the fix for:
  https://bugs.openjdk.java.net/browse/JDK-8032223


Open webrev:

http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8032223-JVMTI-FRAME.1/

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

src/share/vm/prims/jvmtiEnv.cpp
    No comments beyond David's tweak to the comment.

Will fix it before push.

For future work...

Looks like these VM ops also need the liveness check on the
target JavaThread:

    VM_GetOwnedMonitorInfo
    VM_GetStackTrace

JvmtiEnv::GetStackTrace() looks like it has the same
is_thread_fully_suspended() flaw...

I know about this.
There are even more issues:
  GetCurrentContendedMonitor
  GetOwnedMonitorStackDepthInfo
  NotifyFramePop

I'll check if we can use an existing bug to add this extra request.
Otherwise, will file new one.

Filed this one:
  https://bugs.openjdk.java.net/browse/JDK-8034249

Thanks,
Serguei

Thanks,
Serguei

Dan

Summary:

  This is the second round of review for this issue.

But it was decided that the JDK-8032223 must be used to cover it instead of the JDK-6471769. The 8032223 was initially closed as a dup of 6471769 but it has been re-open now.

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 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 major 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.

I've also decided to fix the same issue in the JVMTI GetFrameLocation() as well.

  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 the non-suspended target thread. 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 should be 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.

The bug 6280037 should go out of the Test Stabilization radar (remove the svc-nightly label)

  as the most of the impacted tests must be covered by the 8032223.


Testing:
  In progress:
    - nsk.jvmti, nsk.jdi, nsk.jdwp
    - JTreg com/sun/jdi


Thanks,
Serguei