Fix handling of repeated loads of native libraries with same basename

Loading the same library (identified by its resource path) twice should
succeed, but the second load should be a no-op so that static
initializers and JNI_OnLoad aren't run twice.

Loading two libraries sharing a basename from different paths should
result in an error as this situation is not handled consistently by all
operating systems and determining whether two libraries are truly
identical for all purposes is difficult.

jni/tools/native_loader/BUILD.bazel

@@ -1,5 +1,5 @@
-java_library(
+alias(
     name = "native_loader",
-    srcs = glob(["src/main/java/com/github/fmeum/rules_jni/*.java"]),
+    actual = "//jni/tools/native_loader/src/main/java/com/github/fmeum/rules_jni:native_loader",
     visibility = ["//visibility:public"],
 )

jni/tools/native_loader/src/main/java/com/github/fmeum/rules_jni/BUILD.bazel

@@ -0,0 +1,16 @@
+java_library(
+    name = "native_loader",
+    srcs = [
+        "OsCpuUtils.java",
+        "RulesJni.java",
+    ],
+    visibility = ["//jni/tools/native_loader:__pkg__"],
+    deps = [
+        ":native_library_info",
+    ],
+)
+
+java_library(
+    name = "native_library_info",
+    srcs = ["NativeLibraryInfo.java"],
+)

jni/tools/native_loader/src/main/java/com/github/fmeum/rules_jni/NativeLibraryInfo.java

@@ -0,0 +1,27 @@
+// Copyright 2022 Fabian Meumertzheim
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.github.fmeum.rules_jni;
+
+import java.io.File;
+
+class NativeLibraryInfo {
+  public final String canonicalPath;
+  public final File tempFile;
+
+  NativeLibraryInfo(String canonicalPath, File tempFile) {
+    this.canonicalPath = canonicalPath;
+    this.tempFile = tempFile;
+  }
+}

jni/tools/native_loader/src/main/java/com/github/fmeum/rules_jni/RulesJni.java

@@ -14,20 +14,26 @@
 
 package com.github.fmeum.rules_jni;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.StandardCopyOption;
+import java.nio.file.*;
+import java.util.*;
 
 /**
  * Static helper methods that load native libraries created with the {@code cc_jni_library} rule of
  * <a href="https://github.com/fmeum/rules_jni">{@code rules_jni}</a>.
  */
-public class RulesJni {
+public final class RulesJni {
+  private static final Map<String, NativeLibraryInfo> LOADED_LIBS = new HashMap<>();
+
   private static Path tempDir;
 
+  static {
+    Runtime.getRuntime().addShutdownHook(new Thread(RulesJni::atExit));
+  }
+
   private RulesJni() {}
 
   /**
@@ -54,7 +60,7 @@ private RulesJni() {}
   public static void loadLibrary(String name, Class<?> inSamePackageAs) {
     URL libraryResource = inSamePackageAs.getResource(libraryRelativePath(name));
     failOnNullResource(libraryResource, name);
-    loadLibrary(libraryResource);
+    loadLibrary(name, libraryResource);
   }
 
   /**
@@ -84,18 +90,25 @@ public static void loadLibrary(String name, String absolutePathToPackage) {
     URL libraryResource =
         RulesJni.class.getResource(absolutePathToPackage + "/" + libraryRelativePath(name));
     failOnNullResource(libraryResource, name);
-    loadLibrary(libraryResource);
+    loadLibrary(name, libraryResource);
   }
 
-  synchronized private static void loadLibrary(URL libraryResource) {
+  synchronized private static void loadLibrary(String name, URL libraryResource) {
+    if (LOADED_LIBS.containsKey(name)) {
+      if (!libraryResource.toString().equals(LOADED_LIBS.get(name).canonicalPath)) {
+        throw new UnsatisfiedLinkError(String.format(
+            "Cannot load two native libraries with same basename ('%s') from different paths\nFirst library: %s\nSecond library: %s\n",
+            name, LOADED_LIBS.get(name).canonicalPath, libraryResource));
+      }
+      return;
+    }
     try {
       Path tempDir = getOrCreateTempDir();
       Path tempFile = Files.createTempFile(tempDir, null, null);
+      LOADED_LIBS.put(name, new NativeLibraryInfo(libraryResource.toString(), tempFile.toFile()));
       try (InputStream in = libraryResource.openStream()) {
         Files.copy(in, tempFile, StandardCopyOption.REPLACE_EXISTING);
         System.load(tempFile.toAbsolutePath().toString());
-      } finally {
-        tempFile.toFile().deleteOnExit();
       }
     } catch (IOException e) {
       throw new UnsatisfiedLinkError(e.getMessage());
@@ -105,7 +118,6 @@ synchronized private static void loadLibrary(URL libraryResource) {
   private static Path getOrCreateTempDir() throws IOException {
     if (tempDir == null) {
       tempDir = Files.createTempDirectory("rules_jni.");
-      tempDir.toFile().deleteOnExit();
     }
     return tempDir;
   }
@@ -120,10 +132,17 @@ private static String libraryRelativePath(String name) {
 
   private static void failOnNullResource(URL resource, String name) {
     if (resource == null) {
-      throw new UnsatisfiedLinkError(
-          String.format("Can't find native library '%s' for OS '%s' (\"%s\") and CPU '%s' (\"%s\")",
-              name, OsCpuUtils.CANONICAL_OS, OsCpuUtils.VERBOSE_OS, OsCpuUtils.CANONICAL_CPU,
-              OsCpuUtils.VERBOSE_CPU));
+      throw new UnsatisfiedLinkError(String.format(
+          "Failed to find native library '%s' for OS '%s' (\"%s\") and CPU '%s' (\"%s\")", name,
+          OsCpuUtils.CANONICAL_OS, OsCpuUtils.VERBOSE_OS, OsCpuUtils.CANONICAL_CPU,
+          OsCpuUtils.VERBOSE_CPU));
+    }
+  }
+
+  private static void atExit() {
+    LOADED_LIBS.values().stream().map(l -> l.tempFile).forEach(File::delete);
+    if (tempDir != null) {
+      tempDir.toFile().delete();
     }
   }
 }

tests/native_loader/src/main/java/com/example/math/NativeMath.java

@@ -21,7 +21,7 @@ public class NativeMath {
   static {
     RulesJni.loadLibrary("math", NativeMath.class);
     // Verify that loading the library twice does not result in errors.
-    RulesJni.loadLibrary("math", NativeMath.class);
+    RulesJni.loadLibrary("math", "/com/example/math");
   }
 
   @Native private final static int incrementBy = 1;

tests/native_loader/src/main/java/com/example/os/OsUtils.java

@@ -23,7 +23,7 @@ public class OsUtils {
     String packagePath = OsUtils.class.getPackage().getName().replace(".", "/");
     RulesJni.loadLibrary("os", "/" + packagePath);
     // Verify that loading the library twice does not result in errors.
-    RulesJni.loadLibrary("os", "/" + packagePath);
+    RulesJni.loadLibrary("os", OsUtils.class);
   }
 
   public static native int setenv(String name, String value);

tests/native_loader/src/main/native/com/example/os/onload.cpp

@@ -29,13 +29,24 @@ jint JNI_OnLoad(JavaVM* vm, void* reserved) {
     env->ExceptionDescribe();
     exit(1);
   }
-  jfieldID has_jni_on_load_been_called =
+  jfieldID has_jni_on_load_been_called_field =
       env->GetStaticFieldID(os_utils, "hasJniOnLoadBeenCalled", "Z");
   if (env->ExceptionCheck()) {
     env->ExceptionDescribe();
     exit(1);
   }
-  env->SetStaticBooleanField(os_utils, has_jni_on_load_been_called, true);
+  jboolean has_jni_on_load_been_called =
+      env->GetStaticBooleanField(os_utils, has_jni_on_load_been_called_field);
+  if (env->ExceptionCheck()) {
+    env->ExceptionDescribe();
+    exit(1);
+  }
+  if (has_jni_on_load_been_called) {
+    std::cerr << "JNI_OnLoad of native library 'os' has been called twice"
+              << std::endl;
+    exit(1);
+  }
+  env->SetStaticBooleanField(os_utils, has_jni_on_load_been_called_field, true);
   if (env->ExceptionCheck()) {
     env->ExceptionDescribe();
     exit(1);