From 8c86815a1a4f2b4cfd14c20020e65506762c492b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 29 Dec 2019 17:09:44 -0800 Subject: [PATCH 1/3] Added SpotlessCache.clearOnce, and used it to fix #243 --- .../com/diffplug/spotless/SpotlessCache.java | 29 ++++++++++++++++++- .../gradle/spotless/SpotlessPlugin.java | 12 +++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/SpotlessCache.java b/lib/src/main/java/com/diffplug/spotless/SpotlessCache.java index 0a038db93c..d984107e74 100644 --- 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 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/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java index 43874e1f35..80fcd9c4e3 100644 --- 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,17 @@ 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) + SpotlessCache.clearOnce(project.getRootProject()); + // if we hang onto the rootProject forever, that's a decent-size memory leak + // by setting to null, we force a second clear, but we prevent + project.getGradle().buildFinished(result -> { + SpotlessCache.clearOnce(null); + }); + }); project.afterEvaluate(unused -> { // Add our check task as a dependency on the global check task From 4091e3ff57797da00402a64d569950f89c2f310c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 30 Dec 2019 22:54:43 -0800 Subject: [PATCH 2/3] A better solution. --- .../com/diffplug/gradle/spotless/SpotlessPlugin.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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 index 80fcd9c4e3..4d0be9cb53 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java @@ -40,12 +40,9 @@ public void apply(Project project) { // 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) - SpotlessCache.clearOnce(project.getRootProject()); - // if we hang onto the rootProject forever, that's a decent-size memory leak - // by setting to null, we force a second clear, but we prevent - project.getGradle().buildFinished(result -> { - SpotlessCache.clearOnce(null); - }); + // + // 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 -> { From 12433728d3f9f3e2d2a6ee634be1dd793ba12c6f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 30 Dec 2019 23:08:16 -0800 Subject: [PATCH 3/3] Update changelog. --- plugin-gradle/CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 979979e44f..f3801fa46c 100644 --- 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))