Merge pull request #501 from diffplug/feat/fix-parallel

Fix bug when `gradle clean spotlessCheck` was called on parallel build
diffplug · Jan 1, 2020 · 402be65 · 402be65
2 parents 6e06eda + 1243372
commit 402be65
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 3 deletions.
diff --git a/lib/src/main/java/com/diffplug/spotless/SpotlessCache.java b/lib/src/main/java/com/diffplug/spotless/SpotlessCache.java
@@ -25,6 +25,8 @@
 import java.util.Map;
 import java.util.Objects;
 
+import javax.annotation.Nullable;
+
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 /**
@@ -73,7 +75,12 @@ static SpotlessCache instance() {
 		return instance;
 	}
 
-	/** Closes all cached classloaders. */
+	/**
+	 * Closes all cached classloaders.
+	 *
+	 * @deprecated because it is [dangerous](https://github.com/diffplug/spotless/issues/243#issuecomment-564323856), replacement is {@link #clearOnce(Object)}.
+	 */
+	@Deprecated
 	public static void clear() {
 		List<URLClassLoader> toDelete;
 		synchronized (instance) {
@@ -89,5 +96,25 @@ public static void clear() {
 		}
 	}
 
+	private static volatile Object lastClear;
+
+	/**
+	 * Closes all cached classloaders iff `key` is not `.equals()` to the last call to `clearOnce()`.
+	 * If `key` is null, the clear will always happen (as though null != null).
+	 */
+	public static boolean clearOnce(@Nullable Object key) {
+		synchronized (instance) {
+			if (key == null) {
+				lastClear = null;
+			} else if (key.equals(lastClear)) {
+				// only clear once
+				return false;
+			}
+			lastClear = key;
+		}
+		clear();
+		return true;
+	}
+
 	private static final SpotlessCache instance = new SpotlessCache();
 }
diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md
@@ -2,7 +2,8 @@
 
 ### Version 3.27.0-SNAPSHOT - TBD ([javadoc](https://diffplug.github.io/spotless/javadoc/snapshot/), [snapshot](https://oss.sonatype.org/content/repositories/snapshots/com/diffplug/spotless/spotless-plugin-gradle/))
 
-* Added method `FormatExtension.createIndependentTask(String taskName)` which allows creating a Spotless task outside of the `check`/`apply` lifecycle.  See [javadoc](https://github.com/diffplug/spotless/blob/91ed7203994e52058ea6c2e0f88d023ed290e433/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java#L613-L639) for details.
+* Added method `FormatExtension.createIndependentTask(String taskName)` which allows creating a Spotless task outside of the `check`/`apply` lifecycle.  See [javadoc](https://github.com/diffplug/spotless/blob/91ed7203994e52058ea6c2e0f88d023ed290e433/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java#L613-L639) for details. ([#500](https://github.com/diffplug/spotless/pull/500))
+* Running clean and spotlessCheck during a parallel build could cause exceptions, fixed by ([#501](https://github.com/diffplug/spotless/pull/501)).
 
 ### Version 3.26.1 - November 27th 2019 ([javadoc](https://diffplug.github.io/spotless/javadoc/spotless-plugin-gradle/3.26.1/), [jcenter](https://bintray.com/diffplug/opensource/spotless-plugin-gradle/3.26.1))
 

diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java
@@ -36,7 +36,14 @@ public void apply(Project project) {
 
 		// clear spotless' cache when the user does a clean
 		Task clean = project.getTasks().getByName(BasePlugin.CLEAN_TASK_NAME);
-		clean.doLast(unused -> SpotlessCache.clear());
+		clean.doLast(unused -> {
+			// resolution for: https://github.com/diffplug/spotless/issues/243#issuecomment-564323856
+			// project.getRootProject() is consistent across every project, so only of one the clears will
+			// actually happen (as desired)
+			//
+			// we use System.identityHashCode() to avoid a memory leak by hanging on to the reference directly
+			SpotlessCache.clearOnce(System.identityHashCode(project.getRootProject()));
+		});
 
 		project.afterEvaluate(unused -> {
 			// Add our check task as a dependency on the global check task