From d7e0d88b9cb92dff5be7f944dd2ba46142146100 Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Wed, 30 Sep 2026 19:07:22 -0700 Subject: [PATCH 1/2] fix: load the bundled native library only once per class loader Reloading unpacked the library to a new temporary file, which the JVM loads as a second copy with uninitialized native state. JNI methods could then bind to that copy and fail with JAVA_VM not initialized. Closes #6096 --- .../java/org/apache/comet/NativeBase.java | 10 +++++++++ .../CometSparkSessionExtensionsSuite.scala | 22 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/spark/src/main/java/org/apache/comet/NativeBase.java b/spark/src/main/java/org/apache/comet/NativeBase.java index cc375a03a9b..3d3af017154 100644 --- a/spark/src/main/java/org/apache/comet/NativeBase.java +++ b/spark/src/main/java/org/apache/comet/NativeBase.java @@ -49,6 +49,10 @@ public abstract class NativeBase { private static final String libraryToLoad = System.mapLibraryName(NATIVE_LIB_NAME); private static boolean loaded = false; + // Whether the bundled libcomet has been unpacked and loaded. Unlike `loaded`, never reset: + // unpacking it again yields a new temporary file, which the JVM loads as a second library with + // its own uninitialized native state, and JNI methods can then bind to either copy. + private static boolean bundledLibraryLoaded = false; private static volatile Throwable loadErr = null; private static final String searchPattern = "libcomet-"; private static final AtomicBoolean released = new AtomicBoolean(false); @@ -113,6 +117,11 @@ static synchronized void load() { * Use the bundled native libraries. Functionally equivalent to System.loadLibrary. */ private static void bundleLoadLibrary() { + if (bundledLibraryLoaded) { + loaded = true; + return; + } + String resourceName = resourceName(); InputStream is = NativeBase.class.getResourceAsStream(resourceName); if (is == null) { @@ -133,6 +142,7 @@ private static void bundleLoadLibrary() { Files.copy(is, tempLib.toPath(), StandardCopyOption.REPLACE_EXISTING); System.load(tempLib.getAbsolutePath()); loaded = true; + bundledLibraryLoaded = true; } catch (IOException e) { throw new IllegalStateException("Cannot unpack libcomet: " + e); } finally { diff --git a/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala b/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala index 1e359017a0e..463932461d5 100644 --- a/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala @@ -114,6 +114,28 @@ class CometSparkSessionExtensionsSuite extends CometTestBase { } } + test("reloading NativeBase does not load a second copy of the native library") { + // The bundled library is unpacked to a new temporary file each time it is loaded. To the JVM + // a second copy is a distinct library with its own uninitialized native state, and a JNI + // method first called afterwards can bind to it (#6096). + def unpackedLibraries(): Set[String] = + Option(new java.io.File(System.getProperty("java.io.tmpdir")).list()) + .getOrElse(Array.empty[String]) + .filter(name => name.startsWith("libcomet-") && !name.endsWith(".lck")) + .toSet + + val before = unpackedLibraries() + try { + NativeBase.setLoaded(false) + NativeBase.load() + assert(NativeBase.isLoaded) + } finally { + NativeBase.setLoaded(true) + } + val added = unpackedLibraries() -- before + assert(added.isEmpty, s"reload unpacked another copy of the native library: $added") + } + test("Arrow properties") { NativeBase.setLoaded(false) NativeBase.load() From 6ac6e8888c9c72787381d1ee1a6194ef18c91fc1 Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Wed, 30 Sep 2026 21:43:05 -0700 Subject: [PATCH 2/2] test: make the native library reload check process-local Compare the library path NativeBase recorded instead of listing java.io.tmpdir, which other Comet JVMs also write to. --- .../main/java/org/apache/comet/NativeBase.java | 17 +++++++++++------ .../CometSparkSessionExtensionsSuite.scala | 12 +++--------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/spark/src/main/java/org/apache/comet/NativeBase.java b/spark/src/main/java/org/apache/comet/NativeBase.java index 3d3af017154..ab4eee856ba 100644 --- a/spark/src/main/java/org/apache/comet/NativeBase.java +++ b/spark/src/main/java/org/apache/comet/NativeBase.java @@ -49,10 +49,10 @@ public abstract class NativeBase { private static final String libraryToLoad = System.mapLibraryName(NATIVE_LIB_NAME); private static boolean loaded = false; - // Whether the bundled libcomet has been unpacked and loaded. Unlike `loaded`, never reset: - // unpacking it again yields a new temporary file, which the JVM loads as a second library with - // its own uninitialized native state, and JNI methods can then bind to either copy. - private static boolean bundledLibraryLoaded = false; + // The bundled libcomet this class loader unpacked and loaded, or null. Unlike `loaded`, never + // reset: unpacking it again yields a new temporary file, which the JVM loads as a second + // library with its own uninitialized native state, and JNI methods can then bind to either copy. + private static File bundledLibrary = null; private static volatile Throwable loadErr = null; private static final String searchPattern = "libcomet-"; private static final AtomicBoolean released = new AtomicBoolean(false); @@ -80,6 +80,11 @@ static synchronized void setLoaded(boolean b) { loaded = b; } + // Only for testing + static synchronized File bundledLibrary() { + return bundledLibrary; + } + static synchronized void load() { if (loaded) { return; @@ -117,7 +122,7 @@ static synchronized void load() { * Use the bundled native libraries. Functionally equivalent to System.loadLibrary. */ private static void bundleLoadLibrary() { - if (bundledLibraryLoaded) { + if (bundledLibrary != null) { loaded = true; return; } @@ -142,7 +147,7 @@ private static void bundleLoadLibrary() { Files.copy(is, tempLib.toPath(), StandardCopyOption.REPLACE_EXISTING); System.load(tempLib.getAbsolutePath()); loaded = true; - bundledLibraryLoaded = true; + bundledLibrary = tempLib; } catch (IOException e) { throw new IllegalStateException("Cannot unpack libcomet: " + e); } finally { diff --git a/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala b/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala index 463932461d5..92bac5dba86 100644 --- a/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala @@ -118,13 +118,7 @@ class CometSparkSessionExtensionsSuite extends CometTestBase { // The bundled library is unpacked to a new temporary file each time it is loaded. To the JVM // a second copy is a distinct library with its own uninitialized native state, and a JNI // method first called afterwards can bind to it (#6096). - def unpackedLibraries(): Set[String] = - Option(new java.io.File(System.getProperty("java.io.tmpdir")).list()) - .getOrElse(Array.empty[String]) - .filter(name => name.startsWith("libcomet-") && !name.endsWith(".lck")) - .toSet - - val before = unpackedLibraries() + val before = NativeBase.bundledLibrary() try { NativeBase.setLoaded(false) NativeBase.load() @@ -132,8 +126,8 @@ class CometSparkSessionExtensionsSuite extends CometTestBase { } finally { NativeBase.setLoaded(true) } - val added = unpackedLibraries() -- before - assert(added.isEmpty, s"reload unpacked another copy of the native library: $added") + val after = NativeBase.bundledLibrary() + assert(after eq before, s"reload unpacked another copy of the native library: $after") } test("Arrow properties") {