[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.
This commit is contained in:
Kohsuke Kawaguchi committed 2014-07-10 16:30:01 -07:00
1 parent 7b1b50c85a
commit b6226ad2d1
2 files changed
+26 -8

No files matched your search

@@ -11,6 +11,7 @@ import java.util.logging.Logger;
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 final class BuildReference<R> {
* @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
@@ -325,14 +325,21 @@ public abstract class LazyBuildMixIn<JobT extends Job<JobT,RunT> & Queue.Task &
* 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();
}
/**