Skip to content

Commit 0c60cce

Browse files
committed
potential race condition in result aggregation
In attempting to diagnose an ATH failure I noticed what looks like a potential race condition that could lead to results not being correctly updated. For example in rare cases (but maybe not so rare in our ATH) if 2 builds finish at T1, a web request comes in a T2 and then another build fishes at T3 followed by a web request at T4. When T1 arrives the timestamp is set to T1, and then in T2 the current value is checked against the timestamp and set to T1+1 When T3 arrives the timestamp is set to T3. THe issue occurs if you have multiple CPUs and updating the code is fast and T1 = T2 = T3. When the request at T4 arrives the code thinks everything is up-to-date when it is not.
1 parent 5b34858 commit 0c60cce

File tree

1 file changed

+5
-10
lines changed

1 file changed

+5
-10
lines changed

src/main/java/hudson/tasks/test/AggregatedTestResultPublisher.java

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import java.util.Collection;
5454
import java.util.Collections;
5555
import java.util.List;
56+
import java.util.concurrent.atomic.AtomicBoolean;
5657
import jenkins.model.Jenkins;
5758
import net.sf.json.JSONObject;
5859
import org.kohsuke.accmod.Restricted;
@@ -126,13 +127,9 @@ public static final class TestResultAction extends AbstractTestResultAction {
126127
private final boolean includeFailedBuilds;
127128

128129
/**
129-
* The last time the fields of this object is computed from the rest.
130+
* flag set when we need to recompute the aggregation.
130131
*/
131-
private transient long lastUpdated = 0;
132-
/**
133-
* When was the last time any build completed?
134-
*/
135-
private static long lastChanged = 0;
132+
private static final AtomicBoolean RECOMPUTE_NEEDED = new AtomicBoolean(true);
136133

137134
private transient int failCount;
138135
private transient int totalCount;
@@ -271,11 +268,9 @@ public List<AbstractProject> getNoFingerprints() {
271268
justification = "False positive. Short-circuited")
272269
private synchronized void upToDateCheck() {
273270
// up to date check
274-
if (lastUpdated > lastChanged) {
271+
if (!RECOMPUTE_NEEDED.compareAndSet(true, false)) {
275272
return;
276273
}
277-
lastUpdated = lastChanged + 1;
278-
279274
int failCount = 0;
280275
int totalCount = 0;
281276
List<AbstractTestResultAction> individuals = new ArrayList<>();
@@ -352,7 +347,7 @@ public String getUrlName() {
352347
public static class RunListenerImpl extends RunListener<Run> {
353348
@Override
354349
public void onCompleted(Run run, TaskListener listener) {
355-
lastChanged = System.currentTimeMillis();
350+
RECOMPUTE_NEEDED.set(true);
356351
}
357352
}
358353
}

0 commit comments

Comments
 (0)