From 38cff6d2bb502f7d462e1cf8524b9b348b763341 Mon Sep 17 00:00:00 2001 From: modmuss Date: Thu, 28 Nov 2024 15:00:01 +0000 Subject: [PATCH] Make ExceptionUtil & DaemonUtils config cache safe. (#1223) * Make ExceptionUtil & DaemonUtils config cache safe. * Fix tests * Fix build --- .../configuration/CompileConfiguration.java | 3 +- .../loom/task/GenerateSourcesTask.java | 10 +++- .../net/fabricmc/loom/util/ExceptionUtil.java | 15 +++--- .../net/fabricmc/loom/util/ProcessUtil.java | 22 ++++++-- .../loom/util/gradle/daemon/DaemonUtils.java | 52 +++++++++++++++---- .../buildSrc/stopDaemon/TestPlugin.groovy | 2 +- .../loom/test/unit/ProcessUtilTest.groovy | 3 +- 7 files changed, 78 insertions(+), 29 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java index 5cf753ec0..eea3de0bd 100644 --- 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 index 3759cf3a2..f0f087bdb 100644 --- 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 getDaemonUtilsContext(); + // Prevent Gradle from running two gen sources tasks in parallel @ServiceReference(SyncTaskBuildService.NAME) abstract Property 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 index 71f364330..352cb5061 100644 --- 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,7 +59,7 @@ public static E createDescriptiveWrapper(BiFunction getProcessArguments(ProcessHandle handle) { - if (logLevel != LogLevel.INFO && logLevel != LogLevel.DEBUG) { + if (argumentVisibility != ArgumentVisibility.SHOW_SENSITIVE) { return Optional.empty(); } @@ -117,4 +121,14 @@ private Optional 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 index 79ff1bc6b..2ec975838 100644 --- 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 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 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 index 0c885d3c5..b2743e410 100644 --- 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 { 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 index 76a96ea38..055a91f25 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/ProcessUtilTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/ProcessUtilTest.groovy @@ -24,7 +24,6 @@ package net.fabricmc.loom.test.unit -import org.gradle.api.logging.LogLevel import spock.lang.Specification import net.fabricmc.loom.util.ProcessUtil @@ -32,7 +31,7 @@ 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