Fix memory leaks in gradle daemon (#1198)

Author: nedtwigg

CHANGES.md

@@ -12,6 +12,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
 ## [Unreleased]
 ### Fixed
 * Update the `black` version regex to fix `19.10b0` and earlier. (fixes [#1195](https://github.com/diffplug/spotless/issues/1195), regression introduced in `2.25.0`)
+* `GitAttributesLineEndings$RelocatablePolicy` and `FormatterStepImpl` now null-out their initialization lambdas after their state has been calculated, which allows GC to collect variables which were incidentally captured but not needed in the calculated state. ([#1198](https://github.com/diffplug/spotless/pull/1198))
 
 ## [2.25.2] - 2022-05-03
 ### Changes

lib-extra/src/main/java/com/diffplug/spotless/extra/GitAttributesLineEndings.java

@@ -15,8 +15,6 @@
  */
 package com.diffplug.spotless.extra;
 
-import static com.diffplug.spotless.extra.LibExtraPreconditions.requireElementsNonNull;
-
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -78,8 +76,8 @@ public static LineEnding.Policy create(File projectDir, Supplier<Iterable<File>>
 	static class RelocatablePolicy extends LazyForwardingEquality<CachedEndings> implements LineEnding.Policy {
 		private static final long serialVersionUID = 5868522122123693015L;
 
-		final transient File projectDir;
-		final transient Supplier<Iterable<File>> toFormat;
+		transient File projectDir;
+		transient Supplier<Iterable<File>> toFormat;
 
 		RelocatablePolicy(File projectDir, Supplier<Iterable<File>> toFormat) {
 			this.projectDir = Objects.requireNonNull(projectDir, "projectDir");
@@ -88,8 +86,13 @@ static class RelocatablePolicy extends LazyForwardingEquality<CachedEndings> imp
 
 		@Override
 		protected CachedEndings calculateState() throws Exception {
-			Runtime runtime = new RuntimeInit(projectDir, toFormat.get()).atRuntime();
-			return new CachedEndings(projectDir, runtime, toFormat.get());
+			Runtime runtime = new RuntimeInit(projectDir).atRuntime();
+			// LazyForwardingEquality guarantees that this will only be called once, and keeping toFormat
+			// causes a memory leak, see https://github.com/diffplug/spotless/issues/1194
+			CachedEndings state = new CachedEndings(projectDir, runtime, toFormat.get());
+			projectDir = null;
+			toFormat = null;
+			return state;
 		}
 
 		@Override
@@ -146,8 +149,7 @@ static class RuntimeInit {
 		final @Nullable File workTree;
 
 		@SuppressFBWarnings("SIC_INNER_SHOULD_BE_STATIC_ANON")
-		RuntimeInit(File projectDir, Iterable<File> toFormat) {
-			requireElementsNonNull(toFormat);
+		RuntimeInit(File projectDir) {
 			/////////////////////////////////
 			// USER AND SYSTEM-WIDE VALUES //
 			/////////////////////////////////

lib/src/main/java/com/diffplug/spotless/DelegateFormatterStep.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2022 DiffPlug
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.diffplug.spotless;
+
+import java.util.Objects;
+
+/** Superclass of all compound FormatterSteps necessary for {@link com.diffplug.spotless.LazyForwardingEquality#unlazy(java.lang.Object)}. */
+abstract class DelegateFormatterStep implements FormatterStep {
+	protected final FormatterStep delegateStep;
+
+	DelegateFormatterStep(FormatterStep delegateStep) {
+		this.delegateStep = Objects.requireNonNull(delegateStep);
+	}
+
+	@Override
+	public final String getName() {
+		return delegateStep.getName();
+	}
+}

lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2021 DiffPlug
+ * Copyright 2016-2022 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,27 +22,19 @@
 
 import javax.annotation.Nullable;
 
-final class FilterByContentPatternFormatterStep implements FormatterStep {
-	private final FormatterStep delegateStep;
+final class FilterByContentPatternFormatterStep extends DelegateFormatterStep {
 	final Pattern contentPattern;
 
 	FilterByContentPatternFormatterStep(FormatterStep delegateStep, String contentPattern) {
-		this.delegateStep = Objects.requireNonNull(delegateStep);
+		super(delegateStep);
 		this.contentPattern = Pattern.compile(Objects.requireNonNull(contentPattern));
 	}
 
-	@Override
-	public String getName() {
-		return delegateStep.getName();
-	}
-
 	@Override
 	public @Nullable String format(String raw, File file) throws Exception {
 		Objects.requireNonNull(raw, "raw");
 		Objects.requireNonNull(file, "file");
-
 		Matcher matcher = contentPattern.matcher(raw);
-
 		if (matcher.find()) {
 			return delegateStep.format(raw, file);
 		} else {

lib/src/main/java/com/diffplug/spotless/FilterByFileFormatterStep.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 DiffPlug
+ * Copyright 2016-2022 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,20 +20,14 @@
 
 import javax.annotation.Nullable;
 
-final class FilterByFileFormatterStep implements FormatterStep {
-	private final FormatterStep delegateStep;
+final class FilterByFileFormatterStep extends DelegateFormatterStep {
 	private final SerializableFileFilter filter;
 
 	FilterByFileFormatterStep(FormatterStep delegateStep, SerializableFileFilter filter) {
-		this.delegateStep = Objects.requireNonNull(delegateStep);
+		super(delegateStep);
 		this.filter = Objects.requireNonNull(filter);
 	}
 
-	@Override
-	public String getName() {
-		return delegateStep.getName();
-	}
-
 	@Override
 	public @Nullable String format(String raw, File file) throws Exception {
 		Objects.requireNonNull(raw, "raw");

lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2020 DiffPlug
+ * Copyright 2016-2022 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,7 +39,7 @@ abstract class FormatterStepImpl<State extends Serializable> extends Strict<Stat
 	final transient String name;
 
 	/** Transient because only the state matters. */
-	final transient ThrowingEx.Supplier<State> stateSupplier;
+	transient ThrowingEx.Supplier<State> stateSupplier;
 
 	FormatterStepImpl(String name, ThrowingEx.Supplier<State> stateSupplier) {
 		this.name = Objects.requireNonNull(name);
@@ -53,7 +53,11 @@ public String getName() {
 
 	@Override
 	protected State calculateState() throws Exception {
-		return stateSupplier.get();
+		// LazyForwardingEquality guarantees that this will only be called once, and keeping toFormat
+		// causes a memory leak, see https://github.com/diffplug/spotless/issues/1194
+		State state = stateSupplier.get();
+		stateSupplier = null;
+		return state;
 	}
 
 	static final class Standard<State extends Serializable> extends FormatterStepImpl<State> {

lib/src/main/java/com/diffplug/spotless/LazyForwardingEquality.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 DiffPlug
+ * Copyright 2016-2022 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -111,4 +111,18 @@ static byte[] toBytes(Serializable obj) {
 		}
 		return byteOutput.toByteArray();
 	}
+
+	/** Ensures that the lazy state has been evaluated. */
+	public static void unlazy(Object in) {
+		if (in instanceof LazyForwardingEquality) {
+			((LazyForwardingEquality<?>) in).state();
+		} else if (in instanceof DelegateFormatterStep) {
+			unlazy(((DelegateFormatterStep) in).delegateStep);
+		} else if (in instanceof Iterable) {
+			Iterable<Object> cast = (Iterable<Object>) in;
+			for (Object c : cast) {
+				unlazy(c);
+			}
+		}
+	}
 }

plugin-gradle/CHANGES.md

@@ -3,8 +3,11 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`).
 
 ## [Unreleased]
+### Added
+* `FormatExtension.createIndependentApplyTaskLazy`, with same functionality as `createIndependentApplyTaskLazy` but returning `TaskProvider` ([#1198](https://github.com/diffplug/spotless/pull/1198))
 ### Fixed
 * Update the `black` version regex to fix `19.10b0` and earlier. (fixes [#1195](https://github.com/diffplug/spotless/issues/1195), regression introduced in `6.5.0`)
+* Improved daemon memory consumption ([#1198](https://github.com/diffplug/spotless/pull/1198) fixes [#1194](https://github.com/diffplug/spotless/issues/1194))
 
 ## [6.5.2] - 2022-05-03
 ### Changes

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java

@@ -38,6 +38,7 @@
 import org.gradle.api.file.ConfigurableFileTree;
 import org.gradle.api.file.FileCollection;
 import org.gradle.api.plugins.BasePlugin;
+import org.gradle.api.tasks.TaskProvider;
 
 import com.diffplug.common.base.Preconditions;
 import com.diffplug.spotless.FormatExceptionPolicyStrict;
@@ -769,6 +770,11 @@ protected Project getProject() {
 		return spotless.project;
 	}
 
+	/** Eager version of {@link #createIndependentApplyTaskLazy(String)} */
+	public SpotlessApply createIndependentApplyTask(String taskName) {
+		return createIndependentApplyTaskLazy(taskName).get();
+	}
+
 	/**
 	 * Creates an independent {@link SpotlessApply} for (very) unusual circumstances.
 	 *
@@ -782,19 +788,19 @@ protected Project getProject() {
 	 *
 	 * NOTE: does not respect the rarely-used <a href="https://github.com/diffplug/spotless/blob/b7f8c551a97dcb92cc4b0ee665448da5013b30a3/plugin-gradle/README.md#can-i-apply-spotless-to-specific-files">{@code spotlessFiles} property</a>.
 	 */
-	public SpotlessApply createIndependentApplyTask(String taskName) {
+	public TaskProvider<SpotlessApply> createIndependentApplyTaskLazy(String taskName) {
 		Preconditions.checkArgument(!taskName.endsWith(SpotlessExtension.APPLY), "Task name must not end with " + SpotlessExtension.APPLY);
-		// create and setup the task
-		SpotlessTaskImpl spotlessTask = spotless.project.getTasks().create(taskName + SpotlessTaskService.INDEPENDENT_HELPER, SpotlessTaskImpl.class);
-		spotlessTask.init(spotless.getRegisterDependenciesTask().getTaskService());
-		setupTask(spotlessTask);
-		// clean removes the SpotlessCache, so we have to run after clean
-		spotlessTask.mustRunAfter(BasePlugin.CLEAN_TASK_NAME);
+		TaskProvider<SpotlessTaskImpl> spotlessTask = spotless.project.getTasks().register(taskName + SpotlessTaskService.INDEPENDENT_HELPER, SpotlessTaskImpl.class, task -> {
+			task.init(spotless.getRegisterDependenciesTask().getTaskService());
+			setupTask(task);
+			// clean removes the SpotlessCache, so we have to run after clean
+			task.mustRunAfter(BasePlugin.CLEAN_TASK_NAME);
+		});
 		// create the apply task
-		SpotlessApply applyTask = spotless.project.getTasks().create(taskName, SpotlessApply.class);
-		applyTask.init(spotlessTask);
-		applyTask.dependsOn(spotlessTask);
-
+		TaskProvider<SpotlessApply> applyTask = spotless.project.getTasks().register(taskName, SpotlessApply.class, task -> {
+			task.dependsOn(spotlessTask);
+			task.init(spotlessTask.get());
+		});
 		return applyTask;
 	}
 

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java

@@ -127,12 +127,10 @@ private static Provisioner forConfigurationContainer(Project project, Configurat
 				if (!projName.isEmpty()) {
 					projName = projName + "/";
 				}
-				logger.error(
-						"You need to add a repository containing the '{}' artifact in '{}build.gradle'.\n" +
+				throw new GradleException(String.format(
+						"You need to add a repository containing the '%s' artifact in '%sbuild.gradle'.%n" +
 								"E.g.: 'repositories { mavenCentral() }'",
-						mavenCoords, projName,
-						e);
-				throw e;
+						mavenCoords, projName), e);
 			}
 		};
 	}