[rvm-research] soft references

UGAWA Tomoharu <[email protected]>
Newsgroups gmane.comp.java.jikes.rvm.devel
Message-ID <[email protected]>
Hi

I have found that reference types show a strange behaviour.
When a soft reference A is reachable only though another soft
reference B, the garbage collector fails to preserve the correct
semantics.

In the attached program RefRef.java, there are 3 objects.
A "normal" object is directly reachable from a local variable, o. 
A SoftReference object is also directly reachable from a local
variable, rr. Its referent, call it r, is another SoftReference object,
which in turn has as its referent, o. Immediately prior to GC, 
r is soft reachable from rr (and only from rr). Thus, using the 
notation -s-> to indicate a soft reference, and naming objects
by the variables that point to them, immediately before GC we 
have:

root  ---> rr -s-> r -s-> o <--- root

The specification for soft references permits two actions on a
softly reachable (and by definition, not strongly reachable) object:
it preserves the object or it may choose to reclaim it. The only softly
reachable object in this scenario is r.

In the former case, the object graph above is unchanged. Specifically,
r == rr.get() && r.get() == o.
In the latter case, after GC, rr.get() == null.
However, r.get() returns null on Jikes RVM, which should be impossible
since either r == rr.get() == null and we should get an NPE when we call r.get(),
or r == rr.get() != null and r.get() should return o.

This bug seem to be caused by an incorrect processing of
reference table (ReferenceProcessor.references).  The reference
processor retains softly reachable objects (i.e. referents of soft 
references) unless we have insufficient memory.  This is realized 
by scanning the reference table and marking referents of *live* soft 
references. If the reference processor finds a soft reference that 
is not live (not strongly reachable) in the reference table, it 
disposes of this reference, clears the referent field of this
reference (a comment admits that this is paranoia) and removes the
reference from the reference table.

However, in Jikes RVM the disposed reference may still be
reachable from a referent of another, live soft reference.
Consequently, Jikes RVM incorrectly allows a user program to access
the disposed reference and to get its referent (which is now null).

The error does not lie in clearing the referent field of the disposed
(softly reachable) reference. Simply retaining the value of the referent
field would not solve the problem because, (1) the field would not be
updated if the referent were moved by a moving collector and (2) the 
reference would never be enqueued.

In Jikes RVM, ReferenceProcessor is too eager to discard reference objects
that are not marked. Right at the start of 
ReferenceProcessor.processReference(), we have:
    
    /*
     * If the reference is dead, we're done with it. Let it (and
     * possibly its referent) be garbage-collected.
     */
    if (!trace.isLive(reference)) {
      clearReferent(reference);                   // Too much paranoia ...
      if (TRACE_UNREACHABLE) { VM.sysWriteln(" UNREACHABLE reference:  ",reference); }
      if (TRACE_DETAIL) {
        VM.sysWriteln(" (unreachable)");
      }
      return ObjectReference.nullReference();
    } 

This is wrong if we have chosen to preserve soft references
because this object might be reachable from a live soft reference.

A solution is as follows.

1. Usual standard transitive closure, tracing from the usual roots, marking objects.
2a. If we decide to retain softly reachable objects,
        examine each SoftReference in the table of references
        if the reference is marked, mark the transitive closure of its referent.
2b. If we decide to reclaim softly reachable objects,
	examine each SoftReference in the table of references 
	if its referent is not marked, set the referent to null.
3. Reclaim any unmarked objects.

Best
Tomoharu

------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk

_______________________________________________
Jikesrvm-researchers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/jikesrvm-researchers
RefRef.java (application/octet-stream, 445 B) - not displayed
reftype-bugfix.patch (application/octet-stream, 7.7 KB)
diff -r a7626555f445 MMTk/ext/vm/jikesrvm/org/jikesrvm/mm/mmtk/ReferenceProcessor.java
--- a/MMTk/ext/vm/jikesrvm/org/jikesrvm/mm/mmtk/ReferenceProcessor.java	Tue Sep 17 21:54:14 2013 +0200
+++ b/MMTk/ext/vm/jikesrvm/org/jikesrvm/mm/mmtk/ReferenceProcessor.java	Tue Nov 05 18:05:49 2013 +0900
@@ -313,34 +313,41 @@
    * @param nursery Scan only the newly created references
    */
   @Override
-  public void scan(TraceLocal trace, boolean nursery) {
+  public void scan(TraceLocal trace, boolean nursery, boolean retain) {
     unforwardedReferences = references;
 
     if (TRACE) VM.sysWriteln("Starting ReferenceGlue.scan(",semanticsStr,")");
     int toIndex = nursery ? nurseryIndex : 0;
 
-    if (TRACE_DETAIL) VM.sysWriteln(semanticsStr," Reference table is ",Magic.objectAsAddress(references));
-    for (int fromIndex = toIndex; fromIndex < maxIndex; fromIndex++) {
-      ObjectReference reference = getReference(fromIndex);
-
-      /* Determine liveness (and forward if necessary) the reference */
-      ObjectReference newReference = processReference(trace,reference);
-      if (!newReference.isNull()) {
-        setReference(toIndex++,newReference);
-        if (TRACE_DETAIL) {
-          int index = toIndex-1;
-          VM.sysWrite("SCANNED ",index);
-          VM.sysWrite(" ",references.get(index));
-          VM.sysWrite(" -> ");
-          VM.sysWriteln(getReferent(references.get(index).toObjectReference()));
+    if (retain) {
+      for (int fromIndex = toIndex; fromIndex < maxIndex; fromIndex++) {
+        ObjectReference reference = getReference(fromIndex);
+        retainReferent(trace, reference);
+      }
+    } else {
+      if (TRACE_DETAIL) VM.sysWriteln(semanticsStr," Reference table is ",Magic.objectAsAddress(references));
+      for (int fromIndex = toIndex; fromIndex < maxIndex; fromIndex++) {
+        ObjectReference reference = getReference(fromIndex);
+  
+        /* Determine liveness (and forward if necessary) the reference */
+        ObjectReference newReference = processReference(trace,reference);
+        if (!newReference.isNull()) {
+          setReference(toIndex++,newReference);
+          if (TRACE_DETAIL) {
+            int index = toIndex-1;
+            VM.sysWrite("SCANNED ",index);
+            VM.sysWrite(" ",references.get(index));
+            VM.sysWrite(" -> ");
+            VM.sysWriteln(getReferent(references.get(index).toObjectReference()));
+          }
         }
       }
+      if (Options.verbose.getValue() >= 3) {
+        VM.sysWrite(semanticsStr);
+        VM.sysWriteln(" references: ",maxIndex," -> ",toIndex);
+      }
+      nurseryIndex = maxIndex = toIndex;
     }
-    if (Options.verbose.getValue() >= 3) {
-      VM.sysWrite(semanticsStr);
-      VM.sysWriteln(" references: ",maxIndex," -> ",toIndex);
-    }
-    nurseryIndex = maxIndex = toIndex;
 
     /* flush out any remset entries generated during the above activities */
     Selected.Mutator.get().flushRememberedSets();
@@ -407,7 +414,45 @@
    */
 
   /**
-   * Process a reference with the current semantics.
+   * This method deal with only soft references.
+   * Retain the referent if the reference is definitely reachable.
+   * @param reference the address of the reference. This may or may not
+   * be the address of a heap object, depending on the VM.
+   * @param trace the thread local trace element.
+   */
+  @UninterruptibleNoWarn("Call out to ReferenceQueue API")
+  protected void retainReferent(TraceLocal trace, ObjectReference reference) {
+    if (VM.VerifyAssertions) VM._assert(!reference.isNull());
+    if (VM.VerifyAssertions) VM._assert(semantics == Semantics.SOFT);
+
+    if (TRACE_DETAIL) {
+      VM.sysWrite("Processing reference: ",reference);
+    }
+
+    if (!trace.isLive(reference)) {
+      /*
+       * Reference is currently unreachable.  This may get reachable by the following trace.
+       * We postpone the decision.
+       */
+      return;
+    }
+
+    /*
+     * Reference is definitely reachable.  Retain the referent.
+     */
+    ObjectReference referent = getReferent(reference);
+    if (!referent.isNull())
+      trace.retainReferent(referent);
+    if (TRACE_DETAIL) {
+      VM.sysWriteln(" ~> ", referent.toAddress(), " (retained)");
+    }
+  }
+
+  /**
+   * Process a reference.  This method deals with a soft reference as if it were a
+   * weak reference, i.e., this does not retain the referent.
+   * To retain the referent, use retainReferent() followed by a transitive closure
+   * phase.
    * @param reference the address of the reference. This may or may not
    * be the address of a heap object, depending on the VM.
    * @param trace the thread local trace element.
@@ -453,24 +498,6 @@
 
     if (TRACE_DETAIL)  VM.sysWrite(" => ",newReference);
 
-    if (semantics == Semantics.SOFT) {
-      /*
-       * Unless we've completely run out of memory, we keep
-       * softly reachable objects alive.
-       */
-      if (!Plan.isEmergencyCollection()) {
-        if (TRACE_DETAIL) VM.sysWrite(" (soft) ");
-        trace.retainReferent(oldReferent);
-      }
-    } else if (semantics == Semantics.PHANTOM) {
-      /*
-       * The spec says we should forward the reference.  Without unsafe uses of
-       * reflection, the application can't tell the difference whether we do or not,
-       * so we don't forward the reference.
-       */
-//    trace.retainReferent(oldReferent);
-    }
-
     if (trace.isLive(oldReferent)) {
       if (VM.VerifyAssertions) {
         if (!DebugUtil.validRef(oldReferent)) {
diff -r a7626555f445 MMTk/src/org/mmtk/plan/SimpleCollector.java
--- a/MMTk/src/org/mmtk/plan/SimpleCollector.java	Tue Sep 17 21:54:14 2013 +0200
+++ b/MMTk/src/org/mmtk/plan/SimpleCollector.java	Tue Nov 05 18:05:49 2013 +0900
@@ -73,20 +73,23 @@
 
     if (phaseId == Simple.SOFT_REFS) {
       if (primary) {
-        if (Options.noReferenceTypes.getValue())
-          VM.softReferences.clear();
-        else
-          VM.softReferences.scan(getCurrentTrace(),global().isCurrentGCNursery());
+        if (!Options.noReferenceTypes.getValue()) {
+          if (!Plan.isEmergencyCollection())
+            VM.softReferences.scan(getCurrentTrace(),global().isCurrentGCNursery(), true);
+        }
       }
       return;
     }
 
     if (phaseId == Simple.WEAK_REFS) {
       if (primary) {
-        if (Options.noReferenceTypes.getValue())
+        if (Options.noReferenceTypes.getValue()) {
+          VM.softReferences.clear();
           VM.weakReferences.clear();
-        else
-          VM.weakReferences.scan(getCurrentTrace(),global().isCurrentGCNursery());
+        } else {
+          VM.softReferences.scan(getCurrentTrace(),global().isCurrentGCNursery(), false);
+          VM.weakReferences.scan(getCurrentTrace(),global().isCurrentGCNursery(), false);
+        }
       }
       return;
     }
@@ -106,7 +109,7 @@
         if (Options.noReferenceTypes.getValue())
           VM.phantomReferences.clear();
         else
-          VM.phantomReferences.scan(getCurrentTrace(),global().isCurrentGCNursery());
+          VM.phantomReferences.scan(getCurrentTrace(),global().isCurrentGCNursery(), false);
       }
       return;
     }
diff -r a7626555f445 MMTk/src/org/mmtk/vm/ReferenceProcessor.java
--- a/MMTk/src/org/mmtk/vm/ReferenceProcessor.java	Tue Sep 17 21:54:14 2013 +0200
+++ b/MMTk/src/org/mmtk/vm/ReferenceProcessor.java	Tue Nov 05 18:05:49 2013 +0900
@@ -35,8 +35,9 @@
    *
    * @param trace the thread local trace element.
    * @param nursery {@code true} if it is safe to only scan new references.
+   * @param retain whether or not those references whose referents are not reachable should be tried to retain
    */
-  public abstract void scan(TraceLocal trace, boolean nursery);
+  public abstract void scan(TraceLocal trace, boolean nursery, boolean retain);
 
   /**
    * Iterate over all references and forward.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.