Skip to content

Commit 10d085c

Browse files
committed
Fix spotbugs warning on repo cache size updates
Spotbugs correctly reported a race condition where the check for insertion of a value might report the value is missing then another thread inserted the value before the current thread performed the `put`. Simpler to `put` the value every time the method is called and then check for the rare case when the value that was put is smaller than the value that was there perviously. Repository size cache is used as a hint. Errors in the size cache may lead to suboptimal choices temporarily, but they should not lead to incorrect behavior.
1 parent a46f5a3 commit 10d085c

File tree

1 file changed

+8
-10
lines changed

1 file changed

+8
-10
lines changed

src/main/java/jenkins/plugins/git/GitToolChooser.java

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -269,17 +269,15 @@ private boolean setSizeFromInternalCache(String repoURL) {
269269
/** Cache the estimated repository size for variants of repository URL */
270270
private void assignSizeToInternalCache(String repoURL, long repoSize) {
271271
repoURL = convertToCanonicalURL(repoURL);
272-
if (repositorySizeCache.containsKey(repoURL)) {
273-
long oldSize = repositorySizeCache.get(repoURL);
274-
if (oldSize < repoSize) {
275-
LOGGER.log(Level.FINE, "Replacing old repo size {0} with new size {1} for repo {2}", new Object[]{oldSize, repoSize, repoURL});
276-
repositorySizeCache.put(repoURL, repoSize);
277-
} else if (oldSize > repoSize) {
278-
LOGGER.log(Level.FINE, "Ignoring new size {1} in favor of old size {0} for repo {2}", new Object[]{oldSize, repoSize, repoURL});
279-
}
280-
} else {
272+
Long oldSize = repositorySizeCache.put(repoURL, repoSize);
273+
if (oldSize == null) {
281274
LOGGER.log(Level.FINE, "Caching repo size {0} for repo {1}", new Object[]{repoSize, repoURL});
282-
repositorySizeCache.put(repoURL, repoSize);
275+
} else if (oldSize < repoSize) {
276+
LOGGER.log(Level.FINE, "Replaced old repo size {0} with new size {1} for repo {2}", new Object[]{oldSize, repoSize, repoURL});
277+
} else if (oldSize > repoSize) {
278+
/* Put back the larger old repo size and log a warning. This is not harmful but should be quite rare */
279+
LOGGER.log(Level.WARNING, "Ignoring new repo size {1} in favor of old size {0} for repo {2}", new Object[]{oldSize, repoSize, repoURL});
280+
repositorySizeCache.put(repoURL, oldSize);
283281
}
284282
}
285283

0 commit comments

Comments
 (0)