From d3ca087f6b35f8db3417324a62d43790f3d5fc62 Mon Sep 17 00:00:00 2001 From: Carl Mastrangelo Date: Mon, 25 Sep 2017 21:44:12 -0700 Subject: [PATCH] Propagate all exceptions when loading native code Motivation: There are 2 motivations, the first depends on the second: Loading Netty Epoll statically stopped working in 4.1.16, due to `Native` always loading the arch specific shared object. In a static binary, there is no arch specific SO. Second, there are a ton of exceptions that can happen when loading a native library. When loading native code, Netty tries a bunch of different paths but a failure in any given may not be fatal. Additionally: turning on debug logging is not always feasible so exceptions get silently swallowed. Modifications: * Change Epoll and Kqueue to try the static load second * Modify NativeLibraryLoader to record all the locations where exceptions occur. * Attempt to use `addSuppressed` from Java 7 if available. Alternatives Considered: An alternative would be to record log messages at each failure. If all load attempts fail, the log messages are printed as warning, else as debug. The problem with this is there is no `LogRecord` to create like in java.util.logging. Buffering the args to logger.log() at the end of the method loses the call site, and changes the order of events to be confusing. Another alternative is to teach NativeLibraryLoader about loading the SO first, and then the static version. This would consolidate the code fore Epoll, Kqueue, and TCNative. I think this is the long term better option, but this PR is changing a lot already. Someone else can take a crack at it later Results: Epoll Still Loads and easier debugging. --- .../util/internal/NativeLibraryLoader.java | 123 +++++++++++------- .../io/netty/util/internal/ThrowableUtil.java | 45 +++++++ .../java/io/netty/channel/epoll/Native.java | 20 ++- .../java/io/netty/channel/kqueue/Native.java | 21 ++- 4 files changed, 161 insertions(+), 48 deletions(-) diff --git a/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java b/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java index a1ea75fc2f..c286f6f666 100644 --- a/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java +++ b/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java @@ -30,8 +30,10 @@ import java.lang.reflect.Method; import java.net.URL; import java.security.AccessController; import java.security.PrivilegedAction; +import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; +import java.util.List; import java.util.Set; /** @@ -77,16 +79,20 @@ public final class NativeLibraryLoader { * if none of the given libraries load successfully. */ public static void loadFirstAvailable(ClassLoader loader, String... names) { + List suppressed = new ArrayList(); for (String name : names) { try { load(name, loader); return; } catch (Throwable t) { + suppressed.add(t); logger.debug("Unable to load the library '{}', trying next name...", name, t); } } - throw new IllegalArgumentException("Failed to load any of the given libraries: " - + Arrays.toString(names)); + IllegalArgumentException iae = + new IllegalArgumentException("Failed to load any of the given libraries: " + Arrays.toString(names)); + ThrowableUtil.addSuppressedAndClear(iae, suppressed); + throw iae; } /** @@ -112,12 +118,13 @@ public final class NativeLibraryLoader { public static void load(String originalName, ClassLoader loader) { // Adjust expected name to support shading of native libraries. String name = calculatePackagePrefix().replace('.', '_') + originalName; - + List suppressed = new ArrayList(); try { // first try to load from java.library.path loadLibrary(loader, name, false); return; } catch (Throwable ex) { + suppressed.add(ex); logger.debug( "{} cannot be loaded from java.libary.path, " + "now trying export to -Dio.netty.native.workdir: {}", name, WORKDIR, ex); @@ -137,10 +144,14 @@ public final class NativeLibraryLoader { NATIVE_RESOURCE_HOME + "lib" + name + ".jnilib"; url = loader.getResource(fileName); if (url == null) { - throw new FileNotFoundException(fileName); + FileNotFoundException fnf = new FileNotFoundException(fileName); + ThrowableUtil.addSuppressedAndClear(fnf, suppressed); + throw fnf; } } else { - throw new FileNotFoundException(path); + FileNotFoundException fnf = new FileNotFoundException(path); + ThrowableUtil.addSuppressedAndClear(fnf, suppressed); + throw fnf; } } @@ -175,13 +186,17 @@ public final class NativeLibraryLoader { tmpFile.getPath()); } } catch (Throwable t) { + suppressed.add(t); logger.debug("Error checking if {} is on a file store mounted with noexec", tmpFile, t); } // Re-throw to fail the load + ThrowableUtil.addSuppressedAndClear(e, suppressed); throw e; } catch (Exception e) { - throw (UnsatisfiedLinkError) new UnsatisfiedLinkError( - "could not load a native library: " + name).initCause(e); + UnsatisfiedLinkError ule = new UnsatisfiedLinkError("could not load a native library: " + name); + ule.initCause(e); + ThrowableUtil.addSuppressedAndClear(ule, suppressed); + throw ule; } finally { closeQuietly(in); closeQuietly(out); @@ -201,19 +216,29 @@ public final class NativeLibraryLoader { * @param absolute - Whether the native library will be loaded by path or by name */ private static void loadLibrary(final ClassLoader loader, final String name, final boolean absolute) { + Throwable suppressed = null; try { - // Make sure the helper is belong to the target ClassLoader. - final Class newHelper = tryToLoadClass(loader, NativeLibraryUtil.class); - loadLibraryByHelper(newHelper, name, absolute); + try { + // Make sure the helper is belong to the target ClassLoader. + final Class newHelper = tryToLoadClass(loader, NativeLibraryUtil.class); + loadLibraryByHelper(newHelper, name, absolute); + logger.debug("Successfully loaded the library {}", name); + return; + } catch (UnsatisfiedLinkError e) { // Should by pass the UnsatisfiedLinkError here! + suppressed = e; + logger.debug("Unable to load the library '{}', trying other loading mechanism.", name, e); + } catch (Exception e) { + suppressed = e; + logger.debug("Unable to load the library '{}', trying other loading mechanism.", name, e); + } + NativeLibraryUtil.loadLibrary(name, absolute); // Fallback to local helper class. logger.debug("Successfully loaded the library {}", name); - return; - } catch (UnsatisfiedLinkError e) { // Should by pass the UnsatisfiedLinkError here! - logger.debug("Unable to load the library '{}', trying other loading mechanism.", name, e); - } catch (Exception e) { - logger.debug("Unable to load the library '{}', trying other loading mechanism.", name, e); + } catch (UnsatisfiedLinkError ule) { + if (suppressed != null) { + ThrowableUtil.addSuppressed(ule, suppressed); + } + throw ule; } - NativeLibraryUtil.loadLibrary(name, absolute); // Fallback to local helper class. - logger.debug("Successfully loaded the library {}", name); } private static void loadLibraryByHelper(final Class helper, final String name, final boolean absolute) @@ -233,16 +258,15 @@ public final class NativeLibraryLoader { } }); if (ret instanceof Throwable) { - Throwable error = (Throwable) ret; - Throwable cause = error.getCause(); - if (cause != null) { - if (cause instanceof UnsatisfiedLinkError) { - throw (UnsatisfiedLinkError) cause; - } else { - throw new UnsatisfiedLinkError(cause.getMessage()); - } + Throwable t = (Throwable) ret; + assert !(t instanceof UnsatisfiedLinkError) : t + " should be a wrapper throwable"; + Throwable cause = t.getCause(); + if (cause instanceof UnsatisfiedLinkError) { + throw (UnsatisfiedLinkError) cause; } - throw new UnsatisfiedLinkError(error.getMessage()); + UnsatisfiedLinkError ule = new UnsatisfiedLinkError(t.getMessage()); + ule.initCause(t); + throw ule; } } @@ -257,25 +281,36 @@ public final class NativeLibraryLoader { throws ClassNotFoundException { try { return loader.loadClass(helper.getName()); - } catch (ClassNotFoundException e) { - // The helper class is NOT found in target ClassLoader, we have to define the helper class. - final byte[] classBinary = classToByteArray(helper); - return AccessController.doPrivileged(new PrivilegedAction>() { - @Override - public Class run() { - try { - // Define the helper class in the target ClassLoader, - // then we can call the helper to load the native library. - Method defineClass = ClassLoader.class.getDeclaredMethod("defineClass", String.class, - byte[].class, int.class, int.class); - defineClass.setAccessible(true); - return (Class) defineClass.invoke(loader, helper.getName(), classBinary, 0, - classBinary.length); - } catch (Exception e) { - throw new IllegalStateException("Define class failed!", e); + } catch (ClassNotFoundException e1) { + try { + // The helper class is NOT found in target ClassLoader, we have to define the helper class. + final byte[] classBinary = classToByteArray(helper); + return AccessController.doPrivileged(new PrivilegedAction>() { + @Override + public Class run() { + try { + // Define the helper class in the target ClassLoader, + // then we can call the helper to load the native library. + Method defineClass = ClassLoader.class.getDeclaredMethod("defineClass", String.class, + byte[].class, int.class, int.class); + defineClass.setAccessible(true); + return (Class) defineClass.invoke(loader, helper.getName(), classBinary, 0, + classBinary.length); + } catch (Exception e) { + throw new IllegalStateException("Define class failed!", e); + } } - } - }); + }); + } catch (ClassNotFoundException e2) { + ThrowableUtil.addSuppressed(e2, e1); + throw e2; + } catch (RuntimeException e2) { + ThrowableUtil.addSuppressed(e2, e1); + throw e2; + } catch (Error e2) { + ThrowableUtil.addSuppressed(e2, e1); + throw e2; + } } } diff --git a/common/src/main/java/io/netty/util/internal/ThrowableUtil.java b/common/src/main/java/io/netty/util/internal/ThrowableUtil.java index dcb532ec46..4c1cf8fdd5 100644 --- a/common/src/main/java/io/netty/util/internal/ThrowableUtil.java +++ b/common/src/main/java/io/netty/util/internal/ThrowableUtil.java @@ -18,9 +18,26 @@ package io.netty.util.internal; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintStream; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.List; public final class ThrowableUtil { + private static final Method addSupressedMethod = getAddSuppressed(); + + private static Method getAddSuppressed() { + if (PlatformDependent.javaVersion() < 7) { + return null; + } + try { + // addSuppressed is final, so we only need to look it up on Throwable. + return Throwable.class.getDeclaredMethod("addSuppressed", Throwable.class); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + private ThrowableUtil() { } /** @@ -52,4 +69,32 @@ public final class ThrowableUtil { } } } + + public static boolean haveSuppressed() { + return addSupressedMethod != null; + } + + public static void addSuppressed(Throwable target, Throwable suppressed) { + if (!haveSuppressed()) { + return; + } + try { + addSupressedMethod.invoke(target, suppressed); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } catch (InvocationTargetException e) { + throw new RuntimeException(e); + } + } + + public static void addSuppressedAndClear(Throwable target, List suppressed) { + addSuppressed(target, suppressed); + suppressed.clear(); + } + + public static void addSuppressed(Throwable target, List suppressed) { + for (Throwable t : suppressed) { + addSuppressed(target, t); + } + } } diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java index a7c0b45e5c..5ff9ffd911 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java @@ -23,6 +23,8 @@ import io.netty.util.internal.SystemPropertyUtil; import io.netty.channel.unix.FileDescriptor; import io.netty.channel.unix.NativeInetAddress; import io.netty.util.internal.ThrowableUtil; +import io.netty.util.internal.logging.InternalLogger; +import io.netty.util.internal.logging.InternalLoggerFactory; import java.io.IOException; import java.net.InetAddress; @@ -51,6 +53,8 @@ import static io.netty.channel.unix.Errors.newIOException; *

Static members which call JNI methods must be defined in {@link NativeStaticallyReferencedJniMethods}. */ public final class Native { + private static final InternalLogger logger = InternalLoggerFactory.getInstance(Native.class); + static { try { // First, try calling a side-effect free JNI method to see if the library was already @@ -196,8 +200,20 @@ public final class Native { if (!name.startsWith("linux")) { throw new IllegalStateException("Only supported on Linux"); } - NativeLibraryLoader.load("netty_transport_native_epoll_" + PlatformDependent.normalizedArch(), - PlatformDependent.getClassLoader(Native.class)); + String staticLibName = "netty_transport_native_epoll"; + String sharedLibName = staticLibName + '_' + PlatformDependent.normalizedArch(); + ClassLoader cl = PlatformDependent.getClassLoader(Native.class); + try { + NativeLibraryLoader.load(sharedLibName, cl); + } catch (UnsatisfiedLinkError e1) { + try { + NativeLibraryLoader.load(staticLibName, cl); + logger.debug("Failed to load {}", sharedLibName, e1); + } catch (UnsatisfiedLinkError e2) { + ThrowableUtil.addSuppressed(e1, e2); + throw e1; + } + } } private Native() { diff --git a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/Native.java b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/Native.java index c19d3eee17..9a5a1936b5 100644 --- a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/Native.java +++ b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/Native.java @@ -19,6 +19,9 @@ import io.netty.channel.unix.FileDescriptor; import io.netty.util.internal.NativeLibraryLoader; import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.SystemPropertyUtil; +import io.netty.util.internal.ThrowableUtil; +import io.netty.util.internal.logging.InternalLogger; +import io.netty.util.internal.logging.InternalLoggerFactory; import java.io.IOException; import java.util.Locale; @@ -44,6 +47,8 @@ import static io.netty.channel.unix.Errors.newIOException; *

Internal usage only! */ final class Native { + private static final InternalLogger logger = InternalLoggerFactory.getInstance(Native.class); + static { try { // First, try calling a side-effect free JNI method to see if the library was already @@ -111,8 +116,20 @@ final class Native { if (!name.startsWith("mac") && !name.contains("bsd") && !name.startsWith("darwin")) { throw new IllegalStateException("Only supported on BSD"); } - NativeLibraryLoader.load("netty_transport_native_kqueue_" + PlatformDependent.normalizedArch(), - PlatformDependent.getClassLoader(Native.class)); + String staticLibName = "netty_transport_native_kqueue"; + String sharedLibName = staticLibName + '_' + PlatformDependent.normalizedArch(); + ClassLoader cl = PlatformDependent.getClassLoader(Native.class); + try { + NativeLibraryLoader.load(sharedLibName, cl); + } catch (UnsatisfiedLinkError e1) { + try { + NativeLibraryLoader.load(staticLibName, cl); + logger.debug("Failed to load {}", sharedLibName, e1); + } catch (UnsatisfiedLinkError e2) { + ThrowableUtil.addSuppressed(e1, e2); + throw e1; + } + } } private Native() {