changeset 3992:d1bdeef3e3e2

7098282: G1: assert(interval >= 0) failed: Sanity check, referencePolicy.cpp: 76 Summary: There is a race between one thread successfully forwarding and copying the klass mirror for the SoftReference class (including the static master clock) and another thread attempting to use the master clock while attempting to discover a soft reference object. Maintain a shadow copy of the soft reference master clock and use the shadow during reference discovery and reference processing. Reviewed-by: tonyp, brutisso, ysr
author johnc
date Wed, 12 Oct 2011 10:25:51 -0700
parents 3f24f946bc2d
children e4f412d2b75d ec4b032a4977
files src/share/vm/memory/referencePolicy.cpp src/share/vm/memory/referencePolicy.hpp src/share/vm/memory/referenceProcessor.cpp src/share/vm/memory/referenceProcessor.hpp
diffstat 4 files changed, 75 insertions(+), 33 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/memory/referencePolicy.cpp	Tue Oct 11 10:21:36 2011 +0200
+++ b/src/share/vm/memory/referencePolicy.cpp	Wed Oct 12 10:25:51 2011 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -41,8 +41,9 @@
 
 // The oop passed in is the SoftReference object, and not
 // the object the SoftReference points to.
-bool LRUCurrentHeapPolicy::should_clear_reference(oop p) {
-  jlong interval = java_lang_ref_SoftReference::clock() - java_lang_ref_SoftReference::timestamp(p);
+bool LRUCurrentHeapPolicy::should_clear_reference(oop p,
+                                                  jlong timestamp_clock) {
+  jlong interval = timestamp_clock - java_lang_ref_SoftReference::timestamp(p);
   assert(interval >= 0, "Sanity check");
 
   // The interval will be zero if the ref was accessed since the last scavenge/gc.
@@ -71,8 +72,9 @@
 
 // The oop passed in is the SoftReference object, and not
 // the object the SoftReference points to.
-bool LRUMaxHeapPolicy::should_clear_reference(oop p) {
-  jlong interval = java_lang_ref_SoftReference::clock() - java_lang_ref_SoftReference::timestamp(p);
+bool LRUMaxHeapPolicy::should_clear_reference(oop p,
+                                             jlong timestamp_clock) {
+  jlong interval = timestamp_clock - java_lang_ref_SoftReference::timestamp(p);
   assert(interval >= 0, "Sanity check");
 
   // The interval will be zero if the ref was accessed since the last scavenge/gc.
--- a/src/share/vm/memory/referencePolicy.hpp	Tue Oct 11 10:21:36 2011 +0200
+++ b/src/share/vm/memory/referencePolicy.hpp	Wed Oct 12 10:25:51 2011 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -31,19 +31,27 @@
 
 class ReferencePolicy : public CHeapObj {
  public:
-  virtual bool should_clear_reference(oop p)       { ShouldNotReachHere(); return true; }
+  virtual bool should_clear_reference(oop p, jlong timestamp_clock) {
+    ShouldNotReachHere();
+    return true;
+  }
+
   // Capture state (of-the-VM) information needed to evaluate the policy
   virtual void setup() { /* do nothing */ }
 };
 
 class NeverClearPolicy : public ReferencePolicy {
  public:
-  bool should_clear_reference(oop p) { return false; }
+  virtual bool should_clear_reference(oop p, jlong timestamp_clock) {
+    return false;
+  }
 };
 
 class AlwaysClearPolicy : public ReferencePolicy {
  public:
-  bool should_clear_reference(oop p) { return true; }
+  virtual bool should_clear_reference(oop p, jlong timestamp_clock) {
+    return true;
+  }
 };
 
 class LRUCurrentHeapPolicy : public ReferencePolicy {
@@ -55,7 +63,7 @@
 
   // Capture state (of-the-VM) information needed to evaluate the policy
   void setup();
-  bool should_clear_reference(oop p);
+  virtual bool should_clear_reference(oop p, jlong timestamp_clock);
 };
 
 class LRUMaxHeapPolicy : public ReferencePolicy {
@@ -67,7 +75,7 @@
 
   // Capture state (of-the-VM) information needed to evaluate the policy
   void setup();
-  bool should_clear_reference(oop p);
+  virtual bool should_clear_reference(oop p, jlong timestamp_clock);
 };
 
 #endif // SHARE_VM_MEMORY_REFERENCEPOLICY_HPP
--- a/src/share/vm/memory/referenceProcessor.cpp	Tue Oct 11 10:21:36 2011 +0200
+++ b/src/share/vm/memory/referenceProcessor.cpp	Wed Oct 12 10:25:51 2011 -0700
@@ -36,14 +36,19 @@
 ReferencePolicy* ReferenceProcessor::_always_clear_soft_ref_policy = NULL;
 ReferencePolicy* ReferenceProcessor::_default_soft_ref_policy      = NULL;
 bool             ReferenceProcessor::_pending_list_uses_discovered_field = false;
+jlong            ReferenceProcessor::_soft_ref_timestamp_clock = 0;
 
 void referenceProcessor_init() {
   ReferenceProcessor::init_statics();
 }
 
 void ReferenceProcessor::init_statics() {
-  // Initialize the master soft ref clock.
-  java_lang_ref_SoftReference::set_clock(os::javaTimeMillis());
+  jlong now = os::javaTimeMillis();
+
+  // Initialize the soft ref timestamp clock.
+  _soft_ref_timestamp_clock = now;
+  // Also update the soft ref clock in j.l.r.SoftReference
+  java_lang_ref_SoftReference::set_clock(_soft_ref_timestamp_clock);
 
   _always_clear_soft_ref_policy = new AlwaysClearPolicy();
   _default_soft_ref_policy      = new COMPILER2_PRESENT(LRUMaxHeapPolicy())
@@ -57,6 +62,28 @@
   _pending_list_uses_discovered_field = JDK_Version::current().pending_list_uses_discovered_field();
 }
 
+void ReferenceProcessor::enable_discovery(bool verify_disabled, bool check_no_refs) {
+#ifdef ASSERT
+  // Verify that we're not currently discovering refs
+  assert(!verify_disabled || !_discovering_refs, "nested call?");
+
+  if (check_no_refs) {
+    // Verify that the discovered lists are empty
+    verify_no_references_recorded();
+  }
+#endif // ASSERT
+
+  // Someone could have modified the value of the static
+  // field in the j.l.r.SoftReference class that holds the
+  // soft reference timestamp clock using reflection or
+  // Unsafe between GCs. Unconditionally update the static
+  // field in ReferenceProcessor here so that we use the new
+  // value during reference discovery.
+
+  _soft_ref_timestamp_clock = java_lang_ref_SoftReference::clock();
+  _discovering_refs = true;
+}
+
 ReferenceProcessor::ReferenceProcessor(MemRegion span,
                                        bool      mt_processing,
                                        int       mt_processing_degree,
@@ -122,17 +149,21 @@
   // Update (advance) the soft ref master clock field. This must be done
   // after processing the soft ref list.
   jlong now = os::javaTimeMillis();
-  jlong clock = java_lang_ref_SoftReference::clock();
+  jlong soft_ref_clock = java_lang_ref_SoftReference::clock();
+  assert(soft_ref_clock == _soft_ref_timestamp_clock, "soft ref clocks out of sync");
+
   NOT_PRODUCT(
-  if (now < clock) {
-    warning("time warp: %d to %d", clock, now);
+  if (now < _soft_ref_timestamp_clock) {
+    warning("time warp: "INT64_FORMAT" to "INT64_FORMAT,
+            _soft_ref_timestamp_clock, now);
   }
   )
   // In product mode, protect ourselves from system time being adjusted
   // externally and going backward; see note in the implementation of
   // GenCollectedHeap::time_since_last_gc() for the right way to fix
   // this uniformly throughout the VM; see bug-id 4741166. XXX
-  if (now > clock) {
+  if (now > _soft_ref_timestamp_clock) {
+    _soft_ref_timestamp_clock = now;
     java_lang_ref_SoftReference::set_clock(now);
   }
   // Else leave clock stalled at its old value until time progresses
@@ -150,6 +181,16 @@
   // Stop treating discovered references specially.
   disable_discovery();
 
+  // If discovery was concurrent, someone could have modified
+  // the value of the static field in the j.l.r.SoftReference
+  // class that holds the soft reference timestamp clock using
+  // reflection or Unsafe between when discovery was enabled and
+  // now. Unconditionally update the static field in ReferenceProcessor
+  // here so that we use the new value during processing of the
+  // discovered soft refs.
+
+  _soft_ref_timestamp_clock = java_lang_ref_SoftReference::clock();
+
   bool trace_time = PrintGCDetails && PrintReferenceGC;
   // Soft references
   {
@@ -486,7 +527,8 @@
   while (iter.has_next()) {
     iter.load_ptrs(DEBUG_ONLY(!discovery_is_atomic() /* allow_null_referent */));
     bool referent_is_dead = (iter.referent() != NULL) && !iter.is_referent_alive();
-    if (referent_is_dead && !policy->should_clear_reference(iter.obj())) {
+    if (referent_is_dead &&
+        !policy->should_clear_reference(iter.obj(), _soft_ref_timestamp_clock)) {
       if (TraceReferenceGC) {
         gclog_or_tty->print_cr("Dropping reference (" INTPTR_FORMAT ": %s"  ") by policy",
                                iter.obj(), iter.obj()->blueprint()->internal_name());
@@ -1117,7 +1159,7 @@
     // time-stamp policies advance the soft-ref clock only
     // at a major collection cycle, this is always currently
     // accurate.
-    if (!_current_soft_ref_policy->should_clear_reference(obj)) {
+    if (!_current_soft_ref_policy->should_clear_reference(obj, _soft_ref_timestamp_clock)) {
       return false;
     }
   }
--- a/src/share/vm/memory/referenceProcessor.hpp	Tue Oct 11 10:21:36 2011 +0200
+++ b/src/share/vm/memory/referenceProcessor.hpp	Wed Oct 12 10:25:51 2011 -0700
@@ -201,7 +201,6 @@
     assert(_ref != _first_seen, "cyclic ref_list found");
     NOT_PRODUCT(_processed++);
   }
-
 };
 
 class ReferenceProcessor : public CHeapObj {
@@ -209,6 +208,9 @@
   // Compatibility with pre-4965777 JDK's
   static bool _pending_list_uses_discovered_field;
 
+  // The SoftReference master timestamp clock
+  static jlong _soft_ref_timestamp_clock;
+
   MemRegion   _span;                    // (right-open) interval of heap
                                         // subject to wkref discovery
 
@@ -456,19 +458,7 @@
   void      set_span(MemRegion span) { _span = span; }
 
   // start and stop weak ref discovery
-  void enable_discovery(bool verify_disabled, bool check_no_refs) {
-#ifdef ASSERT
-    // Verify that we're not currently discovering refs
-    assert(!verify_disabled || !_discovering_refs, "nested call?");
-
-    if (check_no_refs) {
-      // Verify that the discovered lists are empty
-      verify_no_references_recorded();
-    }
-#endif // ASSERT
-    _discovering_refs = true;
-  }
-
+  void enable_discovery(bool verify_disabled, bool check_no_refs);
   void disable_discovery()  { _discovering_refs = false; }
   bool discovery_enabled()  { return _discovering_refs;  }