Skip to content
Permalink
Browse files
[FIXED JENKINS-19920] Using selfReference to mean “none” from nextBui…
…ld/previousBuild was just asking for trouble.

JENKINS-16194 and an analogous but opposite bug fixed more simply by introducing a separate NONE constant.
Also numberOnDisk did not get updated properly after a build was deleted, causing double loading of build records in some cases.
(cherry picked from commit 7ca4dc4)

Conflicts:
	changelog.html
  • Loading branch information
jglick authored and olivergondza committed Nov 2, 2013
1 parent c56a184 commit 3493bfa273a3d9cc7d22354cc2dc5c8e06bb4667
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 15 deletions.
@@ -162,10 +162,13 @@
* <p>
* Unlike {@link Run}, {@link AbstractBuild}s do lazy-loading, so we don't use
* {@link Run#previousBuild} and {@link Run#nextBuild}, and instead use these
* fields and point to {@link #selfReference} of adjacent builds.
* fields and point to {@link #selfReference} (or {@link #none}) of adjacent builds.
*/
private volatile transient BuildReference<R> previousBuild, nextBuild;

@SuppressWarnings({"unchecked", "rawtypes"}) private static final BuildReference NONE = new BuildReference("NONE", null);
@SuppressWarnings("unchecked") private BuildReference<R> none() {return NONE;}

/*package*/ final transient BuildReference<R> selfReference = new BuildReference<R>(getId(),_this());


@@ -193,11 +196,7 @@ void dropLinks() {
if(nextBuild!=null) {
AbstractBuild nb = nextBuild.get();
if (nb!=null) {
// remove the oldest build
if (previousBuild == selfReference)
nb.previousBuild = nextBuild;
else
nb.previousBuild = previousBuild;
nb.previousBuild = previousBuild;
}
}
if(previousBuild!=null) {
@@ -223,13 +222,11 @@ public R getPreviousBuild() {
this.previousBuild = pb.selfReference;
return pb;
} else {
// this indicates that we know there's no previous build
// (as opposed to we don't know if/what our previous build is.
this.previousBuild = selfReference;
this.previousBuild = none();
return null;
}
}
if (r==selfReference)
if (r==none())
return null;

R referent = r.get();
@@ -253,13 +250,11 @@ public R getNextBuild() {
this.nextBuild = nb.selfReference;
return nb;
} else {
// this indicates that we know there's no next build
// (as opposed to we don't know if/what our next build is.
this.nextBuild = selfReference;
this.nextBuild = none();
return null;
}
}
if (r==selfReference)
if (r==none())
return null;

R referent = r.get();
@@ -723,7 +723,11 @@ protected BuildReference<R> createReference(R r) {

public synchronized boolean removeValue(R run) {
Index copy = copy();
copy.byNumber.remove(getNumberOf(run));
int n = getNumberOf(run);
copy.byNumber.remove(n);
SortedIntList a = new SortedIntList(numberOnDisk);
a.removeValue(n);
numberOnDisk = a;
BuildReference<R> old = copy.byId.remove(getIdOf(run));
this.index = copy;

@@ -38,6 +38,7 @@ import org.jvnet.hudson.test.UnstableBuilder
import static org.junit.Assert.*
import org.junit.Rule
import org.junit.Test
import org.jvnet.hudson.test.Bug

public class AbstractBuildTest {

@@ -121,4 +122,18 @@ public class AbstractBuildTest {
b = j.assertBuildStatus(Result.SUCCESS,p.scheduleBuild2(0).get())
assertCulprits(b,["george"])
}

@Bug(19920)
@Test void lastBuildNextBuild() {
FreeStyleProject p = j.createFreeStyleProject();
AbstractBuild b1 = j.assertBuildStatusSuccess(p.scheduleBuild2(0));
AbstractBuild b2 = j.assertBuildStatusSuccess(p.scheduleBuild2(0));
assertEquals(b2, p.getLastBuild());
b2.getNextBuild(); // force this to be initialized
b2.delete();
assertEquals(b1, p.getLastBuild());
b1 = p.getLastBuild();
assertEquals(null, b1.getNextBuild());
}

}

0 comments on commit 3493bfa

Please sign in to comment.