Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lock jdk creation using file locks #73

Merged
merged 3 commits into from
Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ apply plugin: 'com.palantir.baseline'
apply plugin: 'com.palantir.baseline-java-versions'
apply plugin: 'com.palantir.jdks.latest'

javaVersions {
libraryTarget = 11
runtime = 17
}

version gitVersion()

allprojects {
Expand All @@ -39,9 +44,3 @@ allprojects {
apply plugin: 'org.inferred.processors'
apply plugin: 'com.palantir.java-format'
}

javaVersions {
libraryTarget = 11
runtime = 17
}

5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-73.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Lock jdk creation using file locks
links:
- https://github.com/palantir/gradle-jdks/pull/73
1 change: 1 addition & 0 deletions gradle-jdks/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ dependencies {

implementation 'com.palantir.baseline:gradle-baseline-java'
implementation 'com.palantir.gradle.utils:lazily-configured-mapping'
implementation 'com.google.guava:guava'
CRogers marked this conversation as resolved.
Show resolved Hide resolved

compileOnly 'org.immutables:value::annotations'
annotationProcessor 'org.immutables:value'
Expand Down
53 changes: 50 additions & 3 deletions gradle-jdks/src/main/java/com/palantir/gradle/jdks/JdkManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,25 @@

package com.palantir.gradle.jdks;

import com.google.common.io.Closer;
import com.google.common.util.concurrent.Striped;
import com.palantir.gradle.jdks.JdkPath.Extension;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.IOException;
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.nio.charset.StandardCharsets;
import java.nio.file.AtomicMoveNotSupportedException;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.util.UUID;
import java.util.concurrent.locks.Lock;
import java.util.stream.Stream;
import org.gradle.api.Project;
import org.gradle.api.file.Directory;
Expand All @@ -36,6 +43,7 @@
import org.gradle.process.ExecResult;

public final class JdkManager {

private final Provider<Directory> storageLocation;
private final JdkDistributions jdkDistributions;
private final JdkDownloaders jdkDownloaders;
Expand Down Expand Up @@ -64,9 +72,10 @@ public Path jdk(Project project, JdkSpec jdkSpec) {
.jdkDownloaderFor(project, jdkSpec.distributionName())
.downloadJdkPath(jdkPath);

Path temporaryJdkPath = Paths.get(
diskPath + ".in-progress-" + UUID.randomUUID().toString().substring(0, 8));
try {
Path temporaryJdkPath = diskPath.getParent()
.resolve(diskPath.getFileName() + ".in-progress-"
+ UUID.randomUUID().toString().substring(0, 8));
try (PathLock ignored = new PathLock(diskPath)) {
project.copy(copy -> {
copy.from(unpackTree(project, jdkPath.extension(), jdkArchive));
copy.into(temporaryJdkPath);
Expand All @@ -80,6 +89,8 @@ public Path jdk(Project project, JdkSpec jdkSpec) {

moveJavaHome(javaHome, diskPath);
return diskPath;
} catch (IOException e) {
throw new RuntimeException("Locking failed", e);
} finally {
project.delete(delete -> {
delete.delete(temporaryJdkPath.toFile());
Expand Down Expand Up @@ -171,4 +182,40 @@ private void addCaCert(Project project, Path javaHome, String alias, String caCe
alias, javaHome, output.toString(StandardCharsets.UTF_8)));
}
}

/**
* Abstraction around locking access to a file or directory, by creating another file with a
* matching name, and '.lock' extension. Note that the underlying file locking mechanism is
* left to the filesystem, so we must be careful to work within its bounds. For example,
* POSIX file locks apply to a process, so within the process we must ensure synchronization
* separately.
*/
private static final class PathLock implements Closeable {
private static final Striped<Lock> JVM_LOCKS = Striped.lock(16);
private final Closer closer;

PathLock(Path path) throws IOException {
this.closer = Closer.create();
try {
Lock jvmLock = JVM_LOCKS.get(path);
jvmLock.lock();
closer.register(jvmLock::unlock);
Files.createDirectories(path.getParent());
FileChannel channel = closer.register(FileChannel.open(
path.getParent().resolve(path.getFileName() + ".lock"),
StandardOpenOption.CREATE,
StandardOpenOption.WRITE));
FileLock fileLock = channel.lock();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fancy - didn't know you could get process-level file locks like this. What does it do on windows/NTFS, out of curiosity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On windows, the in-process (non-fs lock) is actually unnecessary since unlike unix, locks aren't owned by entire processes! No reason to remove that lock, though, since it would only increase the code complexity. Ideally we could get windows tests running in #69, however there are some hurdles with pjf+checkstyle

closer.register(fileLock::close);
} catch (Throwable t) {
closer.close();
throw t;
}
}

@Override
public void close() throws IOException {
closer.close();
}
}
}