From c5bab97e18e70d90a5e5f033f5af7ccf81cb9fbd Mon Sep 17 00:00:00 2001 From: Ioannis Tsakpinis Date: Sat, 4 Sep 2021 18:44:49 +0300 Subject: [PATCH] fix(core): System.load calls from incorrect classloader. Close #648 This fix applies to Windows only. Also: - The System.load call will be skipped altogether when the library path does not contain characters above the ASCII range. - SLL now handles more unusual configurations gracefully (some libraries pre-extracted, others in JARs, in any load order). --- doc/notes/3.3.0.md | 1 + .../main/java/org/lwjgl/system/Library.java | 4 +- .../org/lwjgl/system/LibraryResource.java | 2 +- .../org/lwjgl/system/SharedLibraryLoader.java | 98 +++++++++++++------ 4 files changed, 72 insertions(+), 33 deletions(-) diff --git a/doc/notes/3.3.0.md b/doc/notes/3.3.0.md index 75ac4cd328..ff4fa0fad9 100644 --- a/doc/notes/3.3.0.md +++ b/doc/notes/3.3.0.md @@ -92,6 +92,7 @@ This build includes the following changes: - Generator: Native libraries are now initialized before constants. (#630) - Core: Fixed unsafe field reads in `MemoryUtil` setup code. (#632) - Core: The `SharedLibraryLoader` now prepends a `.` to `SHARED_LIBRARY_EXTRACT_DIRECTORY` when the target directory is not temporary. +- Core: The `SharedLibraryLoader` will now load libraries in the correct `ClassLoader` when testing for [JDK-8195129](https://bugs.openjdk.java.net/browse/JDK-8195129). (#648) - EGL: Fixed nullability of `eglInitialize` arguments. - GLFW: Fixed main thread check. Setting `GLFW_CHECK_THREAD0` to `false` is now required for offscreen interop. (#538) - OpenCL: Added missing `errcode_ret` parameter to `clLinkProgram`. (#560) diff --git a/modules/lwjgl/core/src/main/java/org/lwjgl/system/Library.java b/modules/lwjgl/core/src/main/java/org/lwjgl/system/Library.java index dd40c87644..b0e596ff9b 100644 --- a/modules/lwjgl/core/src/main/java/org/lwjgl/system/Library.java +++ b/modules/lwjgl/core/src/main/java/org/lwjgl/system/Library.java @@ -120,7 +120,7 @@ public static void loadSystem( apiLog("\tUsing SharedLibraryLoader..."); } // Extract from classpath and try org.lwjgl.librarypath - try (FileChannel ignored = SharedLibraryLoader.load(name, libName, libURL)) { + try (FileChannel ignored = SharedLibraryLoader.load(name, libName, libURL, load)) { if (loadSystemFromLibraryPath(load, context, module, libName, bundledWithLWJGL)) { return; } @@ -262,7 +262,7 @@ private static SharedLibrary loadNative(Class context, String module, String apiLog("\tUsing SharedLibraryLoader..."); } // Extract from classpath and try org.lwjgl.librarypath - try (FileChannel ignored = SharedLibraryLoader.load(name, libName, libURL)) { + try (FileChannel ignored = SharedLibraryLoader.load(name, libName, libURL, null)) { lib = loadNativeFromLibraryPath(context, module, libName, bundledWithLWJGL); if (lib != null) { return lib; diff --git a/modules/lwjgl/core/src/main/java/org/lwjgl/system/LibraryResource.java b/modules/lwjgl/core/src/main/java/org/lwjgl/system/LibraryResource.java index c0a8b6477b..e2895640c2 100644 --- a/modules/lwjgl/core/src/main/java/org/lwjgl/system/LibraryResource.java +++ b/modules/lwjgl/core/src/main/java/org/lwjgl/system/LibraryResource.java @@ -106,7 +106,7 @@ private static Path load(Class context, String module, String name, boolean b apiLog("\tUsing SharedLibraryLoader..."); } // Extract from classpath and try org.lwjgl.librarypath - try (FileChannel ignored = SharedLibraryLoader.load(name, name, resourceURL)) { + try (FileChannel ignored = SharedLibraryLoader.load(name, name, resourceURL, null)) { path = loadFromLibraryPath(module, name, bundledWithLWJGL); if (path != null) { return path; diff --git a/modules/lwjgl/core/src/main/java/org/lwjgl/system/SharedLibraryLoader.java b/modules/lwjgl/core/src/main/java/org/lwjgl/system/SharedLibraryLoader.java index 59227cfed0..c2fa9176a2 100644 --- a/modules/lwjgl/core/src/main/java/org/lwjgl/system/SharedLibraryLoader.java +++ b/modules/lwjgl/core/src/main/java/org/lwjgl/system/SharedLibraryLoader.java @@ -12,7 +12,9 @@ import java.net.*; import java.nio.channels.*; import java.nio.file.*; +import java.util.*; import java.util.concurrent.locks.*; +import java.util.function.*; import java.util.stream.*; import java.util.zip.*; @@ -38,6 +40,10 @@ final class SharedLibraryLoader { @Nullable private static Path extractPath; + private static HashSet extractPaths = new HashSet<>(4); + + private static boolean checkedJDK8195129; + private SharedLibraryLoader() { } @@ -47,24 +53,36 @@ private SharedLibraryLoader() { * @param name the resource name * @param filename the resource filename * @param resource the classpath {@link URL} were the resource can be found + * @param load should call {@code System::load} in the context of the appropriate ClassLoader * * @return a {@link FileChannel} that has locked the resource file */ - static FileChannel load(String name, String filename, URL resource) { + static FileChannel load(String name, String filename, URL resource, @Nullable Consumer load) { try { Path extractedFile; EXTRACT_PATH_LOCK.lock(); try { if (extractPath != null) { - // Reuse the lwjgl shared library location + // This path is already tested and safe to use extractedFile = extractPath.resolve(filename); } else { - extractedFile = getExtractPath(filename, resource); - // Do not store unless the test for JDK-8195129 has passed - if (Platform.get() != Platform.WINDOWS || workaroundJDK8195129(extractedFile)) { - initExtractPath(extractPath = extractedFile.getParent()); + extractedFile = getExtractPath(filename, resource, load); + + Path parent = extractedFile.getParent(); + // Do not store unless the test for JDK-8195129 has passed. + // This means that in the worst case org.lwjgl.librarypath + // will contain multiple directories. (Windows only) + // ----------------- + // Example scenario: + // ----------------- + // * load lwjgl.dll - already extracted and in classpath (SLL not used) + // * load library with loadNative - extracted to a directory with unicode characters + // * then another with loadSystem - this will hit LoadLibraryA in the JVM, need an ANSI-safe directory. + if (Platform.get() != Platform.WINDOWS || checkedJDK8195129) { + extractPath = parent; } + initExtractPath(parent); } } finally { EXTRACT_PATH_LOCK.unlock(); @@ -77,6 +95,11 @@ static FileChannel load(String name, String filename, URL resource) { } private static void initExtractPath(Path extractPath) { + if (extractPaths.contains(extractPath)) { + return; + } + extractPaths.add(extractPath); + String newLibPath = extractPath.toAbsolutePath().toString(); // Prepend the path in which the libraries were extracted to org.lwjgl.librarypath @@ -96,20 +119,24 @@ private static void initExtractPath(Path extractPath) { * * @return the extracted library */ - private static Path getExtractPath(String filename, URL resource) { + private static Path getExtractPath(String filename, URL resource, @Nullable Consumer load) { + Path root, file; + String override = Configuration.SHARED_LIBRARY_EXTRACT_PATH.get(); if (override != null) { - return Paths.get(override, filename); + file = (root = Paths.get(override)).resolve(filename); + if (canWrite(root, file, resource, load)) { + return file; + } + apiLog(String.format("\tThe path %s is not accessible. Trying other paths.", override)); } String version = Version.getVersion().replace(' ', '-'); - Path root, file; - // Temp directory with username in path file = (root = Paths.get(System.getProperty("java.io.tmpdir"))) .resolve(Paths.get(Configuration.SHARED_LIBRARY_EXTRACT_DIRECTORY.get("lwjgl" + System.getProperty("user.name")), version, filename)); - if (canWrite(root, file, resource)) { + if (canWrite(root, file, resource, load)) { return file; } @@ -117,13 +144,13 @@ private static Path getExtractPath(String filename, URL resource) { // Working directory file = (root = Paths.get("").toAbsolutePath()).resolve(lwjgl_version_filename); - if (canWrite(root, file, resource)) { + if (canWrite(root, file, resource, load)) { return file; } // User home file = (root = Paths.get(System.getProperty("user.home"))).resolve(lwjgl_version_filename); - if (canWrite(root, file, resource)) { + if (canWrite(root, file, resource, load)) { return file; } @@ -132,7 +159,7 @@ private static Path getExtractPath(String filename, URL resource) { String env = System.getenv("SystemRoot"); if (env != null) { file = (root = Paths.get(env, "Temp")).resolve(lwjgl_version_filename); - if (canWrite(root, file, resource)) { + if (canWrite(root, file, resource, load)) { return file; } } @@ -141,7 +168,7 @@ private static Path getExtractPath(String filename, URL resource) { env = System.getenv("SystemDrive"); if (env != null) { file = (root = Paths.get(env + "/")).resolve(Paths.get("Temp").resolve(lwjgl_version_filename)); - if (canWrite(root, file, resource)) { + if (canWrite(root, file, resource, load)) { return file; } } @@ -152,7 +179,7 @@ private static Path getExtractPath(String filename, URL resource) { file = Files.createTempDirectory("lwjgl"); root = file.getParent(); file = file.resolve(filename); - if (canWrite(root, file, resource)) { + if (canWrite(root, file, resource, load)) { return file; } } catch (IOException ignored) { @@ -187,10 +214,10 @@ private static FileChannel extract(Path file, URL resource) throws IOException { } // If file doesn't exist or the CRC doesn't match, extract it to the temp dir. - apiLog(String.format(" Extracting: %s", resource.getPath())); + apiLog(String.format("\tExtracting: %s", resource.getPath())); //noinspection FieldAccessNotGuarded (already inside the lock) if (extractPath == null) { - apiLog(String.format(" to: %s", file)); + apiLog(String.format("\t to: %s", file)); } Files.createDirectories(file.getParent()); @@ -212,13 +239,11 @@ private static FileChannel lock(Path file) { try { FileChannel fc = FileChannel.open(file); - //noinspection resource if (fc.tryLock(0L, Long.MAX_VALUE, true) == null) { if (Configuration.DEBUG_LOADER.get(false)) { apiLog("\tFile is locked by another process, waiting..."); } - //noinspection resource fc.lock(0L, Long.MAX_VALUE, true); // this will block until the file is locked } @@ -254,7 +279,7 @@ private static long crc(InputStream input) throws IOException { * * @return true if the file is writable */ - private static boolean canWrite(Path root, Path file, URL resource) { + private static boolean canWrite(Path root, Path file, URL resource, @Nullable Consumer load) { Path testFile; if (Files.exists(file)) { if (!Files.isWritable(file)) { @@ -276,13 +301,8 @@ private static boolean canWrite(Path root, Path file, URL resource) { Files.write(testFile, new byte[0]); Files.delete(testFile); - if (workaroundJDK8195129(file)) { - // We have full access, the JVM has locked the file, but System.load can still fail if - // the path contains unicode characters, due to JDK-8195129. Test for this here and - // return false if it fails to try other paths. - try (FileChannel ignored = extract(file, resource)) { - System.load(file.toAbsolutePath().toString()); - } + if (load != null && Platform.get() == Platform.WINDOWS) { + workaroundJDK8195129(file, resource, load); } return true; @@ -314,8 +334,26 @@ private static void canWriteCleanup(Path root, Path file) { } } - private static boolean workaroundJDK8195129(Path file) { - return Platform.get() == Platform.WINDOWS && file.toString().endsWith(".dll"); + private static void workaroundJDK8195129(Path file, URL resource, @Nonnull Consumer load) throws Throwable { + String filepath = file.toAbsolutePath().toString(); + if (filepath.endsWith(".dll")) { + boolean mustCheck = false; + for (int i = 0; i < filepath.length(); i++) { + if (0x80 <= filepath.charAt(i)) { + mustCheck = true; + } + } + if (mustCheck) { + // We have full access, the JVM has locked the file, but System.load can still fail if + // the path contains unicode characters, due to JDK-8195129. Test for this here and + // try other paths if it fails. + try (FileChannel ignored = extract(file, resource)) { + load.accept(file.toAbsolutePath().toString()); + } + } + checkedJDK8195129 = true; + } } + } \ No newline at end of file