Merge pull request #353 from gradle/lptr/noexec-from-tmp

Require explicit initialization of native integration
gradle · Jan 30, 2025 · 87f4647 · 87f4647
2 parents 54027ce + d7f5114
commit 87f4647
Show file tree

Hide file tree

Showing 19 changed files with 154 additions and 145 deletions.
diff --git a/native-platform/src/main/java/net/rubygrapefruit/platform/Native.java b/native-platform/src/main/java/net/rubygrapefruit/platform/Native.java
@@ -31,38 +31,43 @@
  */
 @ThreadSafe
 public class Native {
-    private static NativeLibraryLoader loader;
-    private static final Map<Class<?>, Object> integrations = new HashMap<Class<?>, Object>();
+    private final NativeLibraryLoader loader;
+    private final Map<Class<?>, Object> integrations = new HashMap<Class<?>, Object>();
 
-    private Native() {
+    private static Native instance;
+
+    private Native(NativeLibraryLoader loader) {
+        this.loader = loader;
     }
 
     /**
-     * Initialises the native integration, if not already initialized.
+     * Initialises the native integration.
      *
-     * @param extractDir The directory to extract native resources into. May be null, in which case a default is
-     * selected.
+     * @param extractDir The directory to extract native resources into.
      *
      * @throws NativeIntegrationUnavailableException When native integration is not available on the current machine.
-     * @throws NativeException On failure to load the native integration.
+     * @throws NativeException On failure to load the native integration or if the integration has already been initialized.
      */
     @ThreadSafe
-    static public void init(File extractDir) throws NativeIntegrationUnavailableException, NativeException {
+    static public Native init(File extractDir) throws NativeIntegrationUnavailableException, NativeException {
         synchronized (Native.class) {
-            if (loader == null) {
-                Platform platform = Platform.current();
-                try {
-                    loader = new NativeLibraryLoader(platform, new NativeLibraryLocator(extractDir, NativeVersion.VERSION));
-                    loader.load(platform.getLibraryName(), platform.getLibraryVariants());
-                    String nativeVersion = NativeLibraryFunctions.getVersion();
-                    if (!nativeVersion.equals(NativeVersion.VERSION)) {
-                        throw new NativeException(String.format("Unexpected native library version loaded. Expected %s, was %s.", NativeVersion.VERSION, nativeVersion));
-                    }
-                } catch (NativeException e) {
-                    throw e;
-                } catch (Throwable t) {
-                    throw new NativeException("Failed to initialise native integration.", t);
+            if (instance != null) {
+                throw new NativeException("Native integration already initialised.");
+            }
+            Platform platform = Platform.current();
+            try {
+                NativeLibraryLoader loader = new NativeLibraryLoader(platform, new NativeLibraryLocator(extractDir, NativeVersion.VERSION));
+                loader.load(platform.getLibraryName(), platform.getLibraryVariants());
+                String nativeVersion = NativeLibraryFunctions.getVersion();
+                if (!nativeVersion.equals(NativeVersion.VERSION)) {
+                    throw new NativeException(String.format("Unexpected native library version loaded. Expected %s, was %s.", NativeVersion.VERSION, nativeVersion));
                 }
+                instance = new Native(loader);
+                return instance;
+            } catch (NativeException e) {
+                throw e;
+            } catch (Throwable t) {
+                throw new NativeException("Failed to initialise native integration.", t);
             }
         }
     }
@@ -76,10 +81,9 @@ static public void init(File extractDir) throws NativeIntegrationUnavailableExce
      * @throws NativeException On failure to load the native integration.
      */
     @ThreadSafe
-    public static <T extends NativeIntegration> T get(Class<T> type)
-            throws NativeIntegrationUnavailableException, NativeException {
-        init(null);
-        synchronized (Native.class) {
+    public <T extends NativeIntegration> T get(Class<T> type)
+        throws NativeIntegrationUnavailableException, NativeException {
+        synchronized (this) {
             Platform platform = Platform.current();
             Class<? extends T> canonicalType = platform.canonicalise(type);
             Object instance = integrations.get(canonicalType);

diff --git a/native-platform/src/main/java/net/rubygrapefruit/platform/internal/NativeLibraryLocator.java b/native-platform/src/main/java/net/rubygrapefruit/platform/internal/NativeLibraryLocator.java
@@ -18,6 +18,7 @@
 
 import net.rubygrapefruit.platform.NativeException;
 
+import javax.annotation.Nullable;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -36,48 +37,36 @@ public NativeLibraryLocator(File extractDir, String version) {
         this.version = version;
     }
 
+    @Nullable
     public File find(LibraryDef libraryDef) throws IOException {
         String resourceName = String.format("net/rubygrapefruit/platform/%s/%s", libraryDef.platform, libraryDef.name);
-        if (extractDir != null) {
-            File libFile = new File(extractDir, String.format("%s/%s/%s", version, libraryDef.platform, libraryDef.name));
-            File lockFile = new File(libFile.getParentFile(), libFile.getName() + ".lock");
-            lockFile.getParentFile().mkdirs();
-            lockFile.createNewFile();
-            RandomAccessFile lockFileAccess = new RandomAccessFile(lockFile, "rw");
-            try {
-                // Take exclusive lock on lock file
-                FileLock lock = lockFileAccess.getChannel().lock();
-                if (lockFile.length() > 0 && lockFileAccess.readBoolean()) {
-                    // Library has been extracted
-                    return libFile;
-                }
-                URL resource = getClass().getClassLoader().getResource(resourceName);
-                if (resource != null) {
-                    // Extract library and write marker to lock file
-                    libFile.getParentFile().mkdirs();
-                    copy(resource, libFile);
-                    lockFileAccess.seek(0);
-                    lockFileAccess.writeBoolean(true);
-                    return libFile;
-                }
-            } finally {
-                // Also releases lock
-                lockFileAccess.close();
+        File libFile = new File(extractDir, String.format("%s/%s/%s", version, libraryDef.platform, libraryDef.name));
+        File lockFile = new File(libFile.getParentFile(), libFile.getName() + ".lock");
+        lockFile.getParentFile().mkdirs();
+        lockFile.createNewFile();
+        RandomAccessFile lockFileAccess = new RandomAccessFile(lockFile, "rw");
+        try {
+            // Take exclusive lock on lock file
+            FileLock lock = lockFileAccess.getChannel().lock();
+            if (lockFile.length() > 0 && lockFileAccess.readBoolean()) {
+                // Library has been extracted
+                return libFile;
             }
-        } else {
             URL resource = getClass().getClassLoader().getResource(resourceName);
             if (resource != null) {
-                File libFile;
-                File libDir = File.createTempFile("native-platform", "dir");
-                libDir.delete();
-                libDir.mkdirs();
-                libFile = new File(libDir, libraryDef.name);
-                libFile.deleteOnExit();
+                // Extract library and write marker to lock file
+                libFile.getParentFile().mkdirs();
                 copy(resource, libFile);
+                lockFileAccess.seek(0);
+                lockFileAccess.writeBoolean(true);
                 return libFile;
+            } else {
+                return null;
             }
+        } finally {
+            // Also releases lock
+            lockFileAccess.close();
         }
-        return null;
     }
 
     private static void copy(URL source, File dest) {

diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/NativePlatformSpec.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/NativePlatformSpec.groovy
@@ -0,0 +1,20 @@
+package net.rubygrapefruit.platform
+
+import spock.lang.Specification
+
+class NativePlatformSpec extends Specification {
+    private static Native nativeIntegration
+
+    static protected <T> T getIntegration(Class<T> type) {
+        synchronized (NativePlatformSpec) {
+            if (nativeIntegration == null) {
+                def tempRoot = java.nio.file.Paths.get("build/test-outputs")
+                java.nio.file.Files.createDirectories(tempRoot)
+                def cacheDir = java.nio.file.Files.createTempDirectory(tempRoot, "native-platform").toFile()
+                cacheDir.mkdirs()
+                nativeIntegration = Native.init(cacheDir)
+            }
+        }
+        nativeIntegration.get(type)
+    }
+}
diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/ProcessLauncherTest.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/ProcessLauncherTest.groovy
@@ -16,10 +16,8 @@
 
 package net.rubygrapefruit.platform
 
-import spock.lang.Specification
-
-class ProcessLauncherTest extends Specification {
-    final ProcessLauncher launcher = Native.get(ProcessLauncher)
+class ProcessLauncherTest extends NativePlatformSpec {
+    final ProcessLauncher launcher = getIntegration(ProcessLauncher)
 
     def "can start a child process"() {
         def javaHome = System.getProperty("java.home")

diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/ProcessTest.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/ProcessTest.groovy
@@ -20,15 +20,14 @@ import net.rubygrapefruit.platform.testfixture.JavaVersion
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
 import spock.lang.IgnoreIf
-import spock.lang.Specification
 
-class ProcessTest extends Specification {
+class ProcessTest extends NativePlatformSpec {
     @Rule TemporaryFolder tmpDir
-    final Process process = Native.get(Process.class)
+    final Process process = getIntegration(Process)
 
     def "caches process instance"() {
         expect:
-        Native.get(Process.class) == process
+        getIntegration(Process) == process
     }
 
     def "can get PID"() {

diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/SystemInfoTest.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/SystemInfoTest.groovy
@@ -16,17 +16,12 @@
 
 package net.rubygrapefruit.platform
 
-import org.junit.Rule
-import org.junit.rules.TemporaryFolder
-import spock.lang.Specification
-
-class SystemInfoTest extends Specification {
-    @Rule TemporaryFolder tmpDir
-    final SystemInfo systemInfo = Native.get(SystemInfo.class)
+class SystemInfoTest extends NativePlatformSpec {
+    final SystemInfo systemInfo = getIntegration(SystemInfo)
 
     def "caches system info instance"() {
         expect:
-        Native.get(SystemInfo.class) == systemInfo
+        getIntegration(SystemInfo) == systemInfo
     }
 
     def "can query OS details"() {

diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/WindowsRegistryTest.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/WindowsRegistryTest.groovy
@@ -2,11 +2,10 @@ package net.rubygrapefruit.platform
 
 import net.rubygrapefruit.platform.internal.Platform
 import spock.lang.IgnoreIf
-import spock.lang.Specification
 
 @IgnoreIf({!Platform.current().windows})
-class WindowsRegistryTest extends Specification {
-    def windowsRegistry = Native.get(WindowsRegistry)
+class WindowsRegistryTest extends NativePlatformSpec {
+    def windowsRegistry = getIntegration(WindowsRegistry)
 
     def "can read string value"() {
         expect:

diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/file/AbstractFilesTest.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/file/AbstractFilesTest.groovy
@@ -16,18 +16,26 @@
 
 package net.rubygrapefruit.platform.file
 
+import net.rubygrapefruit.platform.NativePlatformSpec
 import net.rubygrapefruit.platform.internal.Platform
-import spock.lang.Specification
 
 import java.nio.file.LinkOption
 import java.nio.file.Paths
 import java.nio.file.attribute.BasicFileAttributeView
 import java.nio.file.attribute.BasicFileAttributes
 import java.nio.file.attribute.PosixFileAttributes
 
-import static java.nio.file.attribute.PosixFilePermission.*
-
-class AbstractFilesTest extends Specification {
+import static java.nio.file.attribute.PosixFilePermission.GROUP_EXECUTE
+import static java.nio.file.attribute.PosixFilePermission.GROUP_READ
+import static java.nio.file.attribute.PosixFilePermission.GROUP_WRITE
+import static java.nio.file.attribute.PosixFilePermission.OTHERS_EXECUTE
+import static java.nio.file.attribute.PosixFilePermission.OTHERS_READ
+import static java.nio.file.attribute.PosixFilePermission.OTHERS_WRITE
+import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE
+import static java.nio.file.attribute.PosixFilePermission.OWNER_READ
+import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE
+
+class AbstractFilesTest extends NativePlatformSpec {
     BasicFileAttributes attributes(File file) {
         return java.nio.file.Files.getFileAttributeView(file.toPath(), BasicFileAttributeView, LinkOption.NOFOLLOW_LINKS).readAttributes()
     }

diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/file/FileSystemsTest.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/file/FileSystemsTest.groovy
@@ -16,16 +16,15 @@
 
 package net.rubygrapefruit.platform.file
 
-import net.rubygrapefruit.platform.Native
+import net.rubygrapefruit.platform.NativePlatformSpec
 import net.rubygrapefruit.platform.internal.Platform
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
 import spock.lang.Requires
-import spock.lang.Specification
 
 import static org.junit.Assume.assumeTrue
 
-class FileSystemsTest extends Specification {
+class FileSystemsTest extends NativePlatformSpec {
     private static final List<String> EXPECTED_FILE_SYSTEM_TYPES = [
         // APFS on macOS
         'apfs',
@@ -43,11 +42,11 @@ class FileSystemsTest extends Specification {
 
     @Rule TemporaryFolder tmpDir
 
-    final FileSystems fileSystems = Native.get(FileSystems.class)
+    final FileSystems fileSystems = getIntegration(FileSystems)
 
     def "caches file systems instance"() {
         expect:
-        Native.get(FileSystems.class) == fileSystems
+        getIntegration(FileSystems) == fileSystems
     }
 
     def "can query filesystem details"() {

diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/file/FilesTest.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/file/FilesTest.groovy
@@ -15,7 +15,6 @@
  */
 package net.rubygrapefruit.platform.file
 
-import net.rubygrapefruit.platform.Native
 import net.rubygrapefruit.platform.internal.Platform
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
@@ -36,7 +35,7 @@ abstract class FilesTest extends AbstractFilesTest {
     ])
     @Rule
     TemporaryFolder tmpDir
-    final def files = Native.get(Files.class)
+    final def files = getIntegration(Files.class)
 
     void assertIsFile(FileInfo stat, File file) {
         assert stat.type == FileInfo.Type.File
@@ -104,7 +103,7 @@ abstract class FilesTest extends AbstractFilesTest {
 
     def "caches file instance"() {
         expect:
-        Native.get(Files.class) == files
+        getIntegration(Files) == files
     }
 
     @Unroll
@@ -269,7 +268,7 @@ abstract class FilesTest extends AbstractFilesTest {
         stat.size == 0
 
         where:
-        fileSystem << Native.get(FileSystems.class).fileSystems
+        fileSystem << getIntegration(FileSystems).fileSystems
     }
 
     @Unroll

diff --git a/native-platform/src/test/groovy/net/rubygrapefruit/platform/file/PosixFilesTest.groovy b/native-platform/src/test/groovy/net/rubygrapefruit/platform/file/PosixFilesTest.groovy
@@ -1,6 +1,5 @@
 package net.rubygrapefruit.platform.file
 
-import net.rubygrapefruit.platform.Native
 import net.rubygrapefruit.platform.NativeException
 import net.rubygrapefruit.platform.internal.Platform
 import spock.lang.IgnoreIf
@@ -11,11 +10,13 @@ import java.nio.file.attribute.PosixFileAttributeView
 import java.nio.file.attribute.PosixFileAttributes
 import java.nio.file.attribute.PosixFilePermission
 
-import static java.nio.file.attribute.PosixFilePermission.*
+import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE
+import static java.nio.file.attribute.PosixFilePermission.OWNER_READ
+import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE
 
 @IgnoreIf({ Platform.current().windows })
 class PosixFilesTest extends FilesTest {
-    final PosixFiles files = Native.get(PosixFiles.class)
+    final PosixFiles files = getIntegration(PosixFiles)
 
     @Override
     void assertIsFile(FileInfo stat, File file) {
@@ -58,8 +59,8 @@ class PosixFilesTest extends FilesTest {
 
     def "uses same instance for specialized file types"() {
         expect:
-        Native.get(PosixFiles.class) == files
-        Native.get(Files.class) == files
+        getIntegration(PosixFiles) == files
+        getIntegration(Files) == files
     }
 
     def "can stat a file with no read permissions"() {