Greetings,

I've updated the "fix" for this bug based on code review comments
received in round 1. I've dropped most of the changes from round 1
with a couple of exceptions.

JDK-8049304 race between VM_Exit and _sync_FutileWakeups->inc()
https://bugs.openjdk.java.net/browse/JDK-8049304

Webrev URL: http://cr.openjdk.java.net/~dcubed/8049304-webrev/2-jdk9-hs-rt/

The easiest way to re-review is to download the two patch files
(round 0 and round 2) and view them in your favorite file merge tool:

http://cr.openjdk.java.net/~dcubed/8049304-webrev/0-jdk9-hs-rt/hotspot.patch
http://cr.openjdk.java.net/~dcubed/8049304-webrev/2-jdk9-hs-rt/hotspot.patch

Testing: Aurora Adhoc RT-SVC nightly batch (in process)
         Aurora Adhoc vm.tmtools batch (in process)
         Kim's repro sequence for JDK-8049304
         Kim's repro sequence for JDK-8129978
         JPRT -testset hotspot

Changes between round 0 and round 2:

- clarify a few comments
- init _has_PerfData flag with '0' (instead of false)
- drop unnecessary use OrderAccess::release_store() to set
  _has_PerfData to '1' (we're in a Mutex)
- change perfMemory_exit() to only call PerfDataManager::destroy()
  when called at a safepoint and when the StatSampler is not
  running; this means when the VM is aborting, we no longer have
  a race between the original crash report and this code path.

Changes between round 1 and round 2:

- clarify a few comments
- drop is_safe parameter to OM_PERFDATA_OP macro
- init _has_PerfData flag with '0' (instead of false)
- drop OrderAccess::fence() call before os::naked_short_sleep() call
- drop PerfDataManager::has_PerfData_with_acquire()
- drop unnecessary use OrderAccess::release_store() to set
  _has_PerfData to '1' (we're in a Mutex)

I believe that I've addressed all comments from round 0 and
from round 1.

Thanks, in advance, for any comments, questions or suggestions.

Dan


On 8/31/15 4:51 PM, Daniel D. Daugherty wrote:
Greetings,

I've updated the "fix" for this bug based on code review comments
received in round 0.

JDK-8049304 race between VM_Exit and _sync_FutileWakeups->inc()
https://bugs.openjdk.java.net/browse/JDK-8049304

Webrev URL: http://cr.openjdk.java.net/~dcubed/8049304-webrev/1-jdk9-hs-rt/

The easiest way to re-review is to download the two patch files
and view them in your favorite file merge tool:

http://cr.openjdk.java.net/~dcubed/8049304-webrev/0-jdk9-hs-rt/hotspot.patch http://cr.openjdk.java.net/~dcubed/8049304-webrev/1-jdk9-hs-rt/hotspot.patch

Testing: Aurora Adhoc RT-SVC nightly batch (in process)
         Aurora Adhoc vm.tmtools batch (in process)
         Kim's repro sequence for JDK-8049304
         Kim's repro sequence for JDK-8129978
         JPRT -testset hotspot

Changes between round 0 and round 1:

- add an 'is_safe' parameter to the OM_PERFDATA_OP macro;
  safepoint-safe callers can access _has_PerfData flag directly;
  non-safepoint-safe callers use a load-acquire to fetch the
  current _has_PerfData flag value
- change PerfDataManager::destroy() to simply set _has_PerfData
  to zero (field is volatile) and then use a fence() to prevent
  any reordering of operations in any direction; it's only done
  once during VM shutdown so...
- change perfMemory_exit() to only call PerfDataManager::destroy()
  when called at a safepoint and when the StatSample is not
  running; this means when the VM is aborting, we no longer have
  a race between the original crash report and this code path.

I believe that I've addressed all comments from round 0.

Thanks, in advance, for any comments, questions or suggestions.

Dan


On 8/25/15 3:08 PM, Daniel D. Daugherty wrote:
Greetings,

I have a "fix" for a long standing race between JVM shutdown and the
JVM statistics subsystem:

JDK-8049304 race between VM_Exit and _sync_FutileWakeups->inc()
https://bugs.openjdk.java.net/browse/JDK-8049304

Webrev URL: http://cr.openjdk.java.net/~dcubed/8049304-webrev/0-jdk9-hs-rt/

Testing: Aurora Adhoc RT-SVC nightly batch
         Aurora Adhoc vm.tmtools batch
         Kim's repro sequence for JDK-8049304
         Kim's repro sequence for JDK-8129978
         JPRT -testset hotspot

This "fix":

- adds a volatile flag to record whether PerfDataManager is holding
  data (PerfData objects)
- adds PerfDataManager::has_PerfData() to return the flag
- changes the Java monitor subsystem's use of PerfData to
  check both allocation of the monitor subsystem specific
  PerfData object and the new PerfDataManager::has_PerfData()
  return value

If the global 'UsePerfData' option is false, the system works as
it did before. If 'UsePerfData' is true (the default on non-embedded
systems), the Java monitor subsystem will allocate a number of
PerfData objects to record information. The objects will record
information about Java monitor subsystem until the JVM shuts down.

When the JVM starts to shutdown, the new PerfDataManager flag will
change to false and the Java monitor subsystem will stop using the
PerfData objects. This is the new behavior. As noted in the comments
I added to the code, the race is still present; I'm just changing
the order and the timing to reduce the likelihood of the crash.

Thanks, in advance, for any comments, questions or suggestions.

Dan







Reply via email to