[FIXED JENKINS-22395] redoing the fix in f1430a2

Based on the last few commits, I proved that the original fix in f1430a2 doesn't really address the problem. That is, once b2 is deleted, and after sufficient garbage collection, we can make b2.previousBuild.get() be null, and then b2.getPreviousBuild().getNextBuild() ends up incorrectly returning b2. In this commit, I roll back that part of f1430a2, and then fix the problem differently. I started thinking that the main problem we are trying to fix here is that the deleted build object should be unreferenceable. That is, it should behave almost as if the object has already been GCed. The easiest way to do this is to clear a BuildReference object, since we always use the same BuildReference object for all inbound references. This change allows us to clear BuildReference. Code like b2.getPreviousBuild() will continue to try to update b1.nextBuildR to b2, but it will only end up wiping out the field, only to have b1.getNextBuild() recompute the correct value. This fix makes both test cases pass in LazyBuildMixInTest. (cherry picked from commit b6226ad)
jenkinsci · Jul 11, 2014 · ad65cec · ad65cec
1 parent 02e9f36
commit ad65cec
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 8 deletions.
diff --git a/core/src/main/java/jenkins/model/lazy/BuildReference.java b/core/src/main/java/jenkins/model/lazy/BuildReference.java
@@ -11,6 +11,7 @@
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import jenkins.model.Jenkins;
+import jenkins.model.lazy.LazyBuildMixIn.RunMixIn;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
@@ -34,7 +35,7 @@ public final class BuildReference<R> {
     private static final Logger LOGGER = Logger.getLogger(BuildReference.class.getName());
 
     final String id;
-    private final Holder<R> holder;
+    private volatile Holder<R> holder;
 
     public BuildReference(String id, R referent) {
         this.id = id;
@@ -47,7 +48,17 @@ public BuildReference(String id, R referent) {
      * @see Holder#get
      */
     public @CheckForNull R get() {
-        return holder.get();
+        Holder<R> h = holder; // capture
+        return h!=null ? h.get() : null;
+    }
+
+    /**
+     * Clear the reference to make a particular R object effectively unreachable.
+     *
+     * @see RunMixIn#dropLinks()
+     */
+    /*package*/ void clear() {
+        holder = null;
     }
 
     @Override

diff --git a/core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java b/core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java
@@ -325,14 +325,21 @@ public final synchronized BuildReference<RunT> createReference() {
          * To implement {@link Run#dropLinks}.
          */
         public final void dropLinks() {
-            RunT nb = getNextBuild();
-            if (nb != null) {
-                nb.getRunMixIn().previousBuildR = previousBuildR;
+            if (nextBuildR != null) {
+                RunT nb = nextBuildR.get();
+                if (nb != null) {
+                    nb.getRunMixIn().previousBuildR = previousBuildR;
+                }
             }
-            RunT pb = getPreviousBuild();
-            if (pb != null) {
-                pb.getRunMixIn().nextBuildR = nextBuildR;
+            if (previousBuildR != null) {
+                RunT pb = previousBuildR.get();
+                if (pb != null) {
+                    pb.getRunMixIn().nextBuildR = nextBuildR;
+                }
             }
+
+            // make this build object unreachable by other Runs
+            createReference().clear();
         }
 
         /**