Make ExceptionUtil & DaemonUtils config cache safe. (#1223)

* Make ExceptionUtil & DaemonUtils config cache safe. * Fix tests * Fix build
FabricMC · Nov 28, 2024 · 38cff6d · 38cff6d
1 parent 758dcb7
commit 38cff6d
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 29 deletions.
diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java
@@ -72,6 +72,7 @@
 import net.fabricmc.loom.util.ProcessUtil;
 import net.fabricmc.loom.util.gradle.GradleUtils;
 import net.fabricmc.loom.util.gradle.SourceSetHelper;
+import net.fabricmc.loom.util.gradle.daemon.DaemonUtils;
 import net.fabricmc.loom.util.service.ScopedServiceFactory;
 import net.fabricmc.loom.util.service.ServiceFactory;
 
@@ -112,7 +113,7 @@ public void run() {
 				extension.setDependencyManager(dependencyManager);
 				dependencyManager.handleDependencies(getProject(), serviceFactory);
 			} catch (Exception e) {
-				ExceptionUtil.processException(e, getProject());
+				ExceptionUtil.processException(e, DaemonUtils.Context.fromProject(getProject()));
 				disownLock();
 				throw ExceptionUtil.createDescriptiveWrapper(RuntimeException::new, "Failed to setup Minecraft", e);
 			}

diff --git a/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java b/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java
@@ -98,6 +98,7 @@
 import net.fabricmc.loom.util.gradle.ThreadedProgressLoggerConsumer;
 import net.fabricmc.loom.util.gradle.ThreadedSimpleProgressLogger;
 import net.fabricmc.loom.util.gradle.WorkerDaemonClientsManagerHelper;
+import net.fabricmc.loom.util.gradle.daemon.DaemonUtils;
 import net.fabricmc.loom.util.ipc.IPCClient;
 import net.fabricmc.loom.util.ipc.IPCServer;
 import net.fabricmc.loom.util.service.ScopedServiceFactory;
@@ -196,6 +197,9 @@ public abstract class GenerateSourcesTask extends AbstractLoomTask {
 	@Inject
 	protected abstract ProgressLoggerFactory getProgressLoggerFactory();
 
+	@Nested
+	protected abstract Property<DaemonUtils.Context> getDaemonUtilsContext();
+
 	// Prevent Gradle from running two gen sources tasks in parallel
 	@ServiceReference(SyncTaskBuildService.NAME)
 	abstract Property<SyncTaskBuildService> getSyncTask();
@@ -250,6 +254,8 @@ public GenerateSourcesTask(DecompilerOptions decompilerOptions) {
 		getMaxCachedFiles().set(GradleUtils.getIntegerPropertyProvider(getProject(), Constants.Properties.DECOMPILE_CACHE_MAX_FILES).orElse(50_000));
 		getMaxCacheFileAge().set(GradleUtils.getIntegerPropertyProvider(getProject(), Constants.Properties.DECOMPILE_CACHE_MAX_AGE).orElse(90));
 
+		getDaemonUtilsContext().set(getProject().getObjects().newInstance(DaemonUtils.Context.class, getProject()));
+
 		mustRunAfter(getProject().getTasks().withType(AbstractRemapJarTask.class));
 	}
 
@@ -267,7 +273,7 @@ public void run() throws IOException {
 			try (var timer = new Timer("Decompiled sources")) {
 				runWithoutCache();
 			} catch (Exception e) {
-				ExceptionUtil.processException(e, getProject());
+				ExceptionUtil.processException(e, getDaemonUtilsContext().get());
 				throw ExceptionUtil.createDescriptiveWrapper(RuntimeException::new, "Failed to decompile", e);
 			}
 
@@ -300,7 +306,7 @@ public void run() throws IOException {
 				runWithCache(fs.getRoot());
 			}
 		} catch (Exception e) {
-			ExceptionUtil.processException(e, getProject());
+			ExceptionUtil.processException(e, getDaemonUtilsContext().get());
 			throw ExceptionUtil.createDescriptiveWrapper(RuntimeException::new, "Failed to decompile", e);
 		}
 	}

diff --git a/src/main/java/net/fabricmc/loom/util/ExceptionUtil.java b/src/main/java/net/fabricmc/loom/util/ExceptionUtil.java
@@ -31,7 +31,6 @@
 import java.util.List;
 import java.util.function.BiFunction;
 
-import org.gradle.api.Project;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -60,27 +59,27 @@ public static <E, C extends Throwable> E createDescriptiveWrapper(BiFunction<Str
 		return constructor.apply(descriptiveMessage, cause);
 	}
 
-	public static void processException(Throwable e, Project project) {
+	public static void processException(Throwable e, DaemonUtils.Context context) {
 		Throwable cause = e;
 		boolean unrecoverable = false;
 
 		while (cause != null) {
 			if (cause instanceof FileSystemUtil.UnrecoverableZipException) {
 				unrecoverable = true;
 			} else if (cause instanceof FileSystemException fse) {
-				printFileLocks(fse.getFile(), project);
+				printFileLocks(fse.getFile());
 				break;
 			}
 
 			cause = cause.getCause();
 		}
 
 		if (unrecoverable) {
-			DaemonUtils.tryStopGradleDaemon(project);
+			DaemonUtils.tryStopGradleDaemon(context);
 		}
 	}
 
-	private static void printFileLocks(String filename, Project project) {
+	private static void printFileLocks(String filename) {
 		final Path path = Paths.get(filename);
 
 		if (!Files.exists(path)) {
@@ -100,13 +99,13 @@ private static void printFileLocks(String filename, Project project) {
 			return;
 		}
 
-		final ProcessUtil processUtil = ProcessUtil.create(project);
+		final ProcessUtil processUtil = ProcessUtil.create(LOGGER.isInfoEnabled() ? ProcessUtil.ArgumentVisibility.SHOW_SENSITIVE : ProcessUtil.ArgumentVisibility.HIDE);
 
 		final String noun = processes.size() == 1 ? "process has" : "processes have";
-		project.getLogger().error("The following {} a lock on the file '{}':", noun, path);
+		LOGGER.error("The following {} a lock on the file '{}':", noun, path);
 
 		for (ProcessHandle process : processes) {
-			project.getLogger().error(processUtil.printWithParents(process));
+			LOGGER.error(processUtil.printWithParents(process));
 		}
 	}
 }
diff --git a/src/main/java/net/fabricmc/loom/util/ProcessUtil.java b/src/main/java/net/fabricmc/loom/util/ProcessUtil.java
@@ -37,18 +37,22 @@
 import net.fabricmc.loom.nativeplatform.LoomNativePlatform;
 import net.fabricmc.loom.nativeplatform.LoomNativePlatformException;
 
-public record ProcessUtil(LogLevel logLevel) {
+public record ProcessUtil(ArgumentVisibility argumentVisibility) {
 	private static final String EXPLORER_COMMAND = "C:\\Windows\\explorer.exe";
 	private static final Logger LOGGER = LoggerFactory.getLogger(ProcessUtil.class);
 
 	public static ProcessUtil create(Project project) {
-		return new ProcessUtil(project.getGradle().getStartParameter().getLogLevel());
+		return create(ArgumentVisibility.get(project));
+	}
+
+	public static ProcessUtil create(ArgumentVisibility argumentVisibility) {
+		return new ProcessUtil(argumentVisibility);
 	}
 
 	public String printWithParents(ProcessHandle handle) {
 		String result = printWithParents(handle, 0).trim();
 
-		if (logLevel != LogLevel.INFO && logLevel != LogLevel.DEBUG) {
+		if (argumentVisibility == ArgumentVisibility.HIDE) {
 			return "Run with --info or --debug to show arguments, may reveal sensitive info\n" + result;
 		}
 
@@ -75,7 +79,7 @@ private String printWithParents(ProcessHandle handle, int depth) {
 	}
 
 	private Optional<String> getProcessArguments(ProcessHandle handle) {
-		if (logLevel != LogLevel.INFO && logLevel != LogLevel.DEBUG) {
+		if (argumentVisibility != ArgumentVisibility.SHOW_SENSITIVE) {
 			return Optional.empty();
 		}
 
@@ -117,4 +121,14 @@ private Optional<String> getWindowTitles(ProcessHandle processHandle) {
 
 		return Optional.of(joiner.toString());
 	}
+
+	public enum ArgumentVisibility {
+		HIDE,
+		SHOW_SENSITIVE;
+
+		static ArgumentVisibility get(Project project) {
+			final LogLevel logLevel = project.getGradle().getStartParameter().getLogLevel();
+			return (logLevel == LogLevel.INFO || logLevel == LogLevel.DEBUG) ? SHOW_SENSITIVE : HIDE;
+		}
+	}
 }
diff --git a/src/main/java/net/fabricmc/loom/util/gradle/daemon/DaemonUtils.java b/src/main/java/net/fabricmc/loom/util/gradle/daemon/DaemonUtils.java
@@ -28,14 +28,16 @@
 import java.util.List;
 import java.util.UUID;
 
+import javax.inject.Inject;
+
 import org.gradle.api.Project;
+import org.gradle.api.provider.Property;
+import org.gradle.api.tasks.Input;
 import org.gradle.cache.FileLockManager;
 import org.gradle.internal.file.Chmod;
 import org.gradle.internal.remote.internal.RemoteConnection;
 import org.gradle.internal.remote.internal.inet.TcpOutgoingConnector;
 import org.gradle.internal.serialize.Serializers;
-import org.gradle.internal.service.ServiceRegistry;
-import org.gradle.invocation.DefaultGradle;
 import org.gradle.launcher.daemon.client.DaemonClientConnection;
 import org.gradle.launcher.daemon.client.StopDispatcher;
 import org.gradle.launcher.daemon.protocol.DaemonMessageSerializer;
@@ -63,17 +65,17 @@ private DaemonUtils() {
 	/**
 	 * Request the Gradle daemon to stop when it becomes idle.
 	 */
-	public static void tryStopGradleDaemon(Project project) {
+	public static void tryStopGradleDaemon(DaemonUtils.Context context) {
 		try {
-			stopWhenIdle(project);
+			stopWhenIdle(context);
 		} catch (Throwable t) {
 			LOGGER.error("Failed to request the Gradle demon to stop", t);
 		}
 	}
 
 	@VisibleForTesting
-	public static boolean stopWhenIdle(Project project) {
-		DaemonInfo daemonInfo = findCurrentDaemon(project);
+	public static boolean stopWhenIdle(DaemonUtils.Context context) {
+		DaemonInfo daemonInfo = findCurrentDaemon(context);
 
 		if (daemonInfo == null) {
 			return false;
@@ -98,14 +100,13 @@ public static boolean stopWhenIdle(Project project) {
 	}
 
 	@Nullable
-	private static DaemonInfo findCurrentDaemon(Project project) {
+	private static DaemonInfo findCurrentDaemon(DaemonUtils.Context context) {
 		// Gradle maintains a list of running daemons in a registry.bin file.
-		final Path registryBin = project.getGradle().getGradleUserHomeDir().toPath().resolve("daemon").resolve(GradleVersion.current().getVersion()).resolve("registry.bin");
-		project.getLogger().lifecycle("Looking for daemon in: " + registryBin);
+		final Path registryBin = Path.of(context.getRegistryBin().get());
+		LOGGER.info("Looking for daemon in: {}", registryBin);
 
 		// We can use a PersistentDaemonRegistry to read this
-		final ServiceRegistry services = ((DefaultGradle) project.getGradle()).getServices();
-		final DaemonRegistry registry = new PersistentDaemonRegistry(registryBin.toFile(), services.get(FileLockManager.class), services.get(Chmod.class));
+		final DaemonRegistry registry = new PersistentDaemonRegistry(registryBin.toFile(), context.getFileLockManager(), context.getChmod());
 
 		final long pid = ProcessHandle.current().pid();
 		final List<DaemonInfo> runningDaemons = registry.getAll();
@@ -121,4 +122,33 @@ private static DaemonInfo findCurrentDaemon(Project project) {
 		LOGGER.warn("Could not find current process in daemon registry: {}", registryBin);
 		return null;
 	}
+
+	public abstract static class Context {
+		@Input
+		protected abstract Property<String> getRegistryBin();
+
+		@Inject
+		protected abstract FileLockManager getFileLockManager();
+
+		@Inject
+		protected abstract Chmod getChmod();
+
+		@SuppressWarnings("unused")
+		@Inject
+		public Context(Project project) {
+			getRegistryBin().set(Context.getRegistryBinPathName(project));
+		}
+
+		public static Context fromProject(Project project) {
+			return project.getObjects().newInstance(Context.class, project);
+		}
+
+		private static String getRegistryBinPathName(Project project) {
+			return project.getGradle().getGradleUserHomeDir().toPath()
+					.resolve("daemon")
+					.resolve(GradleVersion.current().getVersion())
+					.resolve("registry.bin")
+					.toAbsolutePath().toString();
+		}
+	}
 }
diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/stopDaemon/TestPlugin.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/stopDaemon/TestPlugin.groovy
@@ -85,7 +85,7 @@ class TestPlugin implements Plugin<Project> {
 		def future = handler.daemonConnection.thenAccept { it.waitForAndProcessStop() }
 
 		// Stop the daemon
-		def result = DaemonUtils.stopWhenIdle(project)
+		def result = DaemonUtils.stopWhenIdle(DaemonUtils.Context.fromProject(project))
 
 		// Wait for the connection to be processed, this should have already happened, as the above call is blocking
 		future.join()

diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/ProcessUtilTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/ProcessUtilTest.groovy
@@ -24,15 +24,14 @@
 
 package net.fabricmc.loom.test.unit
 
-import org.gradle.api.logging.LogLevel
 import spock.lang.Specification
 
 import net.fabricmc.loom.util.ProcessUtil
 
 class ProcessUtilTest extends Specification {
 	def "print process info"() {
 		when:
-		def output = new ProcessUtil(LogLevel.DEBUG).printWithParents(ProcessHandle.current())
+		def output = new ProcessUtil(ProcessUtil.ArgumentVisibility.SHOW_SENSITIVE).printWithParents(ProcessHandle.current())
 
 		then:
 		// Just a simple check to see if the output is not empty