From 2a385b3e2b015cbf64f59fd29110bdbc287ea570 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Sat, 3 Feb 2024 16:33:28 -0700 Subject: [PATCH] Improve locking strategy for concurrent loom executions (#1031) * Improve locking strategy for concurrent loom executions This is especially useful for when IntelliJ decides to randomly sync the Gradle project while I am running Gradle from the command line already. * Fix style violations * Adjust feedback messages and use Duration for timeout * Fixup message --- .../configuration/CompileConfiguration.java | 117 +++++++++++++++--- 1 file changed, 103 insertions(+), 14 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java index 302c9f93..2d77629d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java @@ -31,11 +31,15 @@ import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; import java.util.function.Consumer; import javax.inject.Inject; +import org.gradle.api.GradleException; import org.gradle.api.Project; +import org.gradle.api.logging.Logger; +import org.gradle.api.logging.Logging; import org.gradle.api.plugins.JavaPlugin; import org.gradle.api.tasks.AbstractCopyTask; import org.gradle.api.tasks.SourceSet; @@ -92,8 +96,10 @@ public abstract class CompileConfiguration implements Runnable { final boolean previousRefreshDeps = extension.refreshDeps(); - if (getAndLock()) { - getProject().getLogger().lifecycle("Found existing cache lock file, rebuilding loom cache. This may have been caused by a failed or canceled build."); + final LockResult lockResult = acquireProcessLockWaiting(getLockFile()); + + if (lockResult != LockResult.ACQUIRED_CLEAN) { + getProject().getLogger().lifecycle("Found existing cache lock file ({}), rebuilding loom cache. This may have been caused by a failed or canceled build.", lockResult); extension.setRefreshDeps(true); } @@ -232,31 +238,114 @@ public abstract class CompileConfiguration implements Runnable { .afterEvaluation(); } - private Path getLockFile() { + private LockFile getLockFile() { final LoomGradleExtension extension = LoomGradleExtension.get(getProject()); final Path cacheDirectory = extension.getFiles().getUserCache().toPath(); final String pathHash = Checksum.projectHash(getProject()); - return cacheDirectory.resolve("." + pathHash + ".lock"); + return new LockFile( + cacheDirectory.resolve("." + pathHash + ".lock"), + "Lock for cache='%s', project='%s'".formatted( + cacheDirectory, getProject().absoluteProjectPath(getProject().getPath()) + ) + ); } - private boolean getAndLock() { - final Path lock = getLockFile(); - - if (Files.exists(lock)) { - return true; + record LockFile(Path file, String description) { + @Override + public String toString() { + return this.description; } + } + enum LockResult { + // acquired immediately or after waiting for another process to release + ACQUIRED_CLEAN, + // already owned by current pid + ACQUIRED_ALREADY_OWNED, + // acquired due to current owner not existing + ACQUIRED_PREVIOUS_OWNER_MISSING + } + + private LockResult acquireProcessLockWaiting(LockFile lockFile) { + // one hour + return this.acquireProcessLockWaiting(lockFile, Duration.ofHours(1)); + } + + private LockResult acquireProcessLockWaiting(LockFile lockFile, Duration timeout) { try { - Files.createFile(lock); - } catch (IOException e) { - throw new UncheckedIOException("Failed to acquire getProject() configuration lock", e); + return this.acquireProcessLockWaiting_(lockFile, timeout); + } catch (final IOException e) { + throw new RuntimeException("Exception acquiring lock " + lockFile, e); + } + } + + // Returns true if our process already owns the lock + @SuppressWarnings("BusyWait") + private LockResult acquireProcessLockWaiting_(LockFile lockFile, Duration timeout) throws IOException { + final long timeoutMs = timeout.toMillis(); + final Logger logger = Logging.getLogger("loom_acquireProcessLockWaiting"); + final long currentPid = ProcessHandle.current().pid(); + boolean abrupt = false; + + if (Files.exists(lockFile.file)) { + long lockingProcessId; + + try { + lockingProcessId = Long.parseLong(Files.readString(lockFile.file)); + } catch (final Exception e) { + lockingProcessId = -1; + } + + if (lockingProcessId == currentPid) { + return LockResult.ACQUIRED_ALREADY_OWNED; + } + + logger.lifecycle("\"{}\" is currently held by pid '{}'.", lockFile, lockingProcessId); + + if (ProcessHandle.of(lockingProcessId).isEmpty()) { + logger.lifecycle("Locking process does not exist, assuming abrupt termination and deleting lock file."); + Files.deleteIfExists(lockFile.file); + abrupt = true; + } else { + logger.lifecycle("Waiting for lock to be released..."); + long sleptMs = 0; + + while (Files.exists(lockFile.file)) { + try { + Thread.sleep(100); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + + sleptMs += 100; + + if (sleptMs >= 1000 * 60 && sleptMs % (1000 * 60) == 0L) { + logger.lifecycle( + """ + Have been waiting on "{}" held by pid '{}' for {} minute(s). + If this persists for an unreasonable length of time, kill this process, run './gradlew --stop' and then try again.""", + lockFile, lockingProcessId, sleptMs / 1000 / 60 + ); + } + + if (sleptMs >= timeoutMs) { + throw new GradleException("Have been waiting on lock file '%s' for %s ms. Giving up as timeout is %s ms." + .formatted(lockFile, sleptMs, timeoutMs)); + } + } + } } - return false; + if (!Files.exists(lockFile.file.getParent())) { + Files.createDirectories(lockFile.file.getParent()); + } + + Files.writeString(lockFile.file, String.valueOf(currentPid)); + return abrupt ? LockResult.ACQUIRED_PREVIOUS_OWNER_MISSING : LockResult.ACQUIRED_CLEAN; } private void releaseLock() { - final Path lock = getLockFile(); + final Path lock = getLockFile().file; if (!Files.exists(lock)) { return;