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.

Sign up for GitHub

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

fix: prevent debug message tearing in multithreaded environments #825

Merged
merged 1 commit into from
Nov 18, 2022
Merged

fix: prevent debug message tearing in multithreaded environments #825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions doc/notes/3.3.2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ This build includes the following changes:
- Core: Fixed Java/native library incompatibility detection. (#737)
- Core: Fixed `dlerror` decoding to use UTF-8. (#738)
- Core: Fixed `Version::getVersion()` when LWJGL is in the module path. (#770)
- Core: Many debug messages can no longer tear under concurrency. (#825)
- Build: Fixed offline mode with multiple local architectures. (#740)
- NanoSVG: Fixed auto-sizing of `NSVGPath::pts` buffer.
- Opus: Fixed `pcm` parameter type of `opus_decode_float` function. (#785)
Expand Down
16 changes: 13 additions & 3 deletions modules/lwjgl/core/src/main/java/org/lwjgl/system/APIUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,20 @@ private APIUtil() {
*
* @param msg the message to print
*/
public static void apiLog(@Nullable CharSequence msg) {
public static void apiLog(CharSequence msg) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure why I marked this as @Nullable in the first place. While the method could technically accept null, we probably don't ever want to do that.

if (DEBUG) {
DEBUG_STREAM.print("[LWJGL] ");
DEBUG_STREAM.println(msg);
DEBUG_STREAM.print("[LWJGL] " + msg + "\n");
}
}

/**
* Same as {@link #apiLog}, but replaces the LWJGL prefix with a tab character.
*
* @param msg the message to print, in continuation of a previous message
*/
public static void apiLogMore(CharSequence msg) {
if (DEBUG) {
DEBUG_STREAM.print("\t" + msg + "\n");
}
}

Expand Down
99 changes: 51 additions & 48 deletions modules/lwjgl/core/src/main/java/org/lwjgl/system/Library.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,12 @@ public final class Library {

static {
if (DEBUG) {
apiLog("Version: " + Version.getVersion());
apiLog("\t OS: " + System.getProperty("os.name") + " v" + System.getProperty("os.version"));
apiLog("\tJRE: " + Platform.get().getName() + " " + System.getProperty("os.arch") + " " + System.getProperty("java.version"));
apiLog(
"\tJVM: " + System.getProperty("java.vm.name") + " v" + System.getProperty("java.vm.version") + " by " + System.getProperty("java.vm.vendor")
DEBUG_STREAM.print(
"[LWJGL] Version: " + Version.getVersion() +
"\n\t OS: " + System.getProperty("os.name") + " v" + System.getProperty("os.version") +
"\n\tJRE: " + Platform.get().getName() + " " + System.getProperty("os.arch") + " " + System.getProperty("java.version") +
"\n\tJVM: " + System.getProperty("java.vm.name") + " v" + System.getProperty("java.vm.version") + " by " + System.getProperty("java.vm.vendor") +
"\n"
);
}

Expand Down Expand Up @@ -84,13 +85,18 @@ public static void loadSystem(
String module,
String name
) throws UnsatisfiedLinkError {
apiLog("Loading JNI library: " + name);
apiLog("\tModule: " + module);
if (DEBUG) {
DEBUG_STREAM.print(
"[LWJGL] Loading JNI library: " + name +
"\n\tModule: " + module +
"\n"
);
}

// METHOD 1: absolute path
if (Paths.get(name).isAbsolute()) {
load.accept(name);
apiLog("\tSuccess");
apiLogMore("Success");
return;
}

Expand All @@ -111,15 +117,15 @@ public static void loadSystem(
String regular = getRegularFilePath(libURL);
if (regular != null) {
load.accept(regular);
apiLog("\tLoaded from classpath: " + regular);
apiLogMore("Loaded from classpath: " + regular);
return;
}
}

// Always use the SLL if the library is found in the classpath,
// so that newer versions can be detected.
if (debugLoader) {
apiLog("\tUsing SharedLibraryLoader...");
apiLogMore("Using SharedLibraryLoader...");
}
// Extract from classpath and try org.lwjgl.librarypath
try (FileChannel ignored = SharedLibraryLoader.load(name, libName, libURL, load)) {
Expand Down Expand Up @@ -151,16 +157,16 @@ public static void loadSystem(
// In that case, ClassLoader::findLibrary was used to return the library path (e.g. OSGi does this with native libraries in bundles).
Path libFile = javaLibraryPath == null ? null : findFile(javaLibraryPath, module, libName, bundledWithLWJGL);
if (libFile != null) {
apiLog(String.format("\tLoaded from %s: %s", JAVA_LIBRARY_PATH, libFile));
apiLogMore(String.format("Loaded from %s: %s", JAVA_LIBRARY_PATH, libFile));
if (bundledWithLWJGL) {
checkHash(context, libFile, module, libName);
}
} else {
apiLog("\tLoaded from a ClassLoader provided path.");
apiLogMore("Loaded from a ClassLoader provided path.");
}
return;
} catch (Throwable t) {
apiLog(String.format("\t%s not found in %s", libName, JAVA_LIBRARY_PATH));
apiLogMore(libName + " not found in " + JAVA_LIBRARY_PATH);
}

detectPlatformMismatch(context, module);
Expand All @@ -176,12 +182,12 @@ private static boolean loadSystemFromLibraryPath(Consumer<String> load, Class<?>
private static boolean loadSystem(Consumer<String> load, Class<?> context, String module, String libName, boolean bundledWithLWJGL, String property, String paths) {
Path libFile = findFile(paths, module, libName, bundledWithLWJGL);
if (libFile == null) {
apiLog(String.format("\t%s not found in %s=%s", libName, property, paths));
apiLogMore(libName + " not found in " + property + "=" + paths);
return false;
}

load.accept(libFile.toAbsolutePath().toString());
apiLog(String.format("\tLoaded from %s: %s", property, libFile));
apiLogMore("Loaded from " + property + ": " + libFile);
if (bundledWithLWJGL) {
checkHash(context, libFile, module, libName);
}
Expand Down Expand Up @@ -232,13 +238,18 @@ public static SharedLibrary loadNative(Class<?> context, String module, String n

@SuppressWarnings("try")
private static SharedLibrary loadNative(Class<?> context, String module, String name, boolean bundledWithLWJGL, boolean printError) {
apiLog("Loading library: " + name);
apiLog("\tModule: " + module);
if (DEBUG) {
DEBUG_STREAM.print(
"[LWJGL] Loading library: " + name +
"\n\tModule: " + module +
"\n"
);
}

// METHOD 1: absolute path
if (Paths.get(name).isAbsolute()) {
SharedLibrary lib = apiCreateLibrary(name);
apiLog("\tSuccess");
apiLogMore("Success");
return lib;
}

Expand All @@ -259,15 +270,15 @@ private static SharedLibrary loadNative(Class<?> context, String module, String
String regular = getRegularFilePath(libURL);
if (regular != null) {
lib = apiCreateLibrary(regular);
apiLog("\tLoaded from classpath: " + regular);
apiLogMore("Loaded from classpath: " + regular);
return lib;
}
}

// Always use the SLL if the library is found in the classpath,
// so that newer versions can be detected.
if (debugLoader) {
apiLog("\tUsing SharedLibraryLoader...");
apiLogMore("Using SharedLibraryLoader...");
}
// Extract from classpath and try org.lwjgl.librarypath
try (FileChannel ignored = SharedLibraryLoader.load(name, libName, libURL, null)) {
Expand Down Expand Up @@ -302,7 +313,7 @@ private static SharedLibrary loadNative(Class<?> context, String module, String
String libPath = (String)findLibrary.invoke(context.getClassLoader(), name);
if (libPath != null) {
lib = apiCreateLibrary(libPath);
apiLog(String.format("\tLoaded from ClassLoader provided path: %s", libPath));
apiLogMore("Loaded from ClassLoader provided path: " + libPath);
return lib;
}
} catch (Exception ignored) {
Expand Down Expand Up @@ -341,12 +352,12 @@ private static SharedLibrary loadNativeFromSystem(String libName) {
try {
lib = apiCreateLibrary(libName);
String path = lib.getPath();
apiLog(path == null
? "\tLoaded from system paths"
: "\tLoaded from system paths: " + path);
apiLogMore(path == null
? "Loaded from system paths"
: "Loaded from system paths: " + path);
} catch (UnsatisfiedLinkError e) {
lib = null;
apiLog(String.format("\t%s not found in system paths", libName));
apiLogMore(libName + " not found in system paths");
}
return lib;
}
Expand All @@ -364,12 +375,12 @@ private static SharedLibrary loadNativeFromLibraryPath(Class<?> context, String
private static SharedLibrary loadNative(Class<?> context, String module, String libName, boolean bundledWithLWJGL, String property, String paths) {
Path libFile = findFile(paths, module, libName, bundledWithLWJGL);
if (libFile == null) {
apiLog(String.format("\t%s not found in %s=%s", libName, property, paths));
apiLogMore(libName + " not found in " + property + "=" + paths);
return null;
}

SharedLibrary lib = apiCreateLibrary(libFile.toAbsolutePath().toString());
apiLog(String.format("\tLoaded from %s: %s", property, libFile));
apiLogMore("Loaded from " + property + ": " + libFile);
if (bundledWithLWJGL) {
checkHash(context, libFile, module, libName);
}
Expand Down Expand Up @@ -518,24 +529,12 @@ private static void detectPlatformMismatch(Class<?> context, String module) {
}

if (!platforms.isEmpty()) {
DEBUG_STREAM.println("[LWJGL] Platform/architecture mismatch detected for module: " + module);
DEBUG_STREAM.println("\tJVM platform:");
DEBUG_STREAM.format(
"\t\t%s %s %s\n",
Platform.get().getName(),
System.getProperty("os.arch"),
System.getProperty("java.version")
);
DEBUG_STREAM.format(
"\t\t%s v%s by %s\n",
System.getProperty("java.vm.name"),
System.getProperty("java.vm.version"),
System.getProperty("java.vm.vendor")
);
DEBUG_STREAM.format(
"\tPlatform%s available on classpath:\n\t\t%s\n",
(platforms.size() == 1 ? "" : "s"),
String.join("\n\t\t", platforms)
DEBUG_STREAM.print(
"[LWJGL] Platform/architecture mismatch detected for module: " + module +
"\n\tJVM platform:" +
"\t\t" + Platform.get().getName() + " " + System.getProperty("os.arch") + " " + System.getProperty("java.version") + "\n" +
"\t\t" + System.getProperty("java.vm.name") + " v" + System.getProperty("java.vm.version") + " by " + System.getProperty("java.vm.vendor") + "\n" +
"\tPlatform" + (platforms.size() == 1 ? "" : "s") + " available on classpath:\n\t\t" + String.join("\n\t\t", platforms) + "\n"
);
}
}
Expand All @@ -552,13 +551,17 @@ private static void printError(boolean bundledWithLWJGL) {
}

static void printError(String message) {
DEBUG_STREAM.println(message);
StringBuilder sb = new StringBuilder(message);
sb.append("\n");

if (!DEBUG) {
DEBUG_STREAM.println("[LWJGL] Enable debug mode with -Dorg.lwjgl.util.Debug=true for better diagnostics.");
sb.append("[LWJGL] Enable debug mode with -Dorg.lwjgl.util.Debug=true for better diagnostics.\n");
if (!Configuration.DEBUG_LOADER.get(false)) {
DEBUG_STREAM.println("[LWJGL] Enable the SharedLibraryLoader debug mode with -Dorg.lwjgl.util.DebugLoader=true for better diagnostics.");
sb.append("[LWJGL] Enable the SharedLibraryLoader debug mode with -Dorg.lwjgl.util.DebugLoader=true for better diagnostics.\n");
}
}

DEBUG_STREAM.print(sb);
}

/**
Expand Down
20 changes: 13 additions & 7 deletions modules/lwjgl/core/src/main/java/org/lwjgl/system/LibraryResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.function.*;

import static org.lwjgl.system.APIUtil.*;
import static org.lwjgl.system.Checks.*;

/**
* Handles loading of native resources in LWJGL. [INTERNAL USE ONLY]
Expand Down Expand Up @@ -68,8 +69,13 @@ public static Path load(Class<?> context, String module, String name, boolean bu

@SuppressWarnings("try")
private static Path load(Class<?> context, String module, String name, boolean bundledWithLWJGL, boolean printError) {
apiLog("Loading library resource: " + name);
apiLog("\tModule: " + module);
if (DEBUG) {
DEBUG_STREAM.print(
"[LWJGL] Loading library resource: " + name +
"\n\tModule: " + module +
"\n"
);
}

// METHOD 1: absolute path
Path path = Paths.get(name);
Expand All @@ -80,7 +86,7 @@ private static Path load(Class<?> context, String module, String name, boolean b
}
throw new IllegalStateException("Failed to locate library resource: " + name);
}
apiLog("\tSuccess");
apiLogMore("Success");
return path;
}

Expand All @@ -96,14 +102,14 @@ private static Path load(Class<?> context, String module, String name, boolean b
try {
String regular = Library.getRegularFilePath(resourceURL);
if (regular != null) {
apiLog("\tLoaded from classpath: " + regular);
apiLogMore("Loaded from classpath: " + regular);
return Paths.get(regular);
}

// Always use the SLL if the resource is found in the classpath,
// so that newer versions can be detected.
if (debugLoader) {
apiLog("\tUsing SharedLibraryLoader...");
apiLogMore("Using SharedLibraryLoader...");
}
// Extract from classpath and try org.lwjgl.librarypath
try (FileChannel ignored = SharedLibraryLoader.load(name, name, resourceURL, null)) {
Expand Down Expand Up @@ -147,11 +153,11 @@ private static Path loadFromLibraryPath(String module, String libName, boolean b
private static Path load(String module, String name, boolean bundledWithLWJGL, String property, String paths) {
Path resource = Library.findFile(paths, module, name, bundledWithLWJGL);
if (resource == null) {
apiLog(String.format("\t%s not found in %s=%s", name, property, paths));
apiLogMore(name + " not found in " + property + "=" + paths);
return null;
}

apiLog(String.format("\tLoaded from %s: %s", property, resource));
apiLogMore("Loaded from " + property + ": " + resource);
return resource;
}

Expand Down
Loading