Repository: netty/netty
Issue: Tcnative init failed with org.apache.tomcat.jni.Library.version(I)I in OSGi
Labels: defect

I am working a OSGi project with netty-tcnative + TLS and encounter a java.lang.UnsatisfiedLinkError in OpenSsl initialization. 

Here is the log:
java.lang.UnsatisfiedLinkError: org.apache.tomcat.jni.Library.version(I)I
    at org.apache.tomcat.jni.Library.version(Native Method)
    at org.apache.tomcat.jni.Library.initialize(Library.java:176)
    at io.netty.handler.ssl.OpenSsl.initializeTcNative(OpenSsl.java:243)
    at io.netty.handler.ssl.OpenSsl.<clinit>(OpenSsl.java:76)

We running a Apache Karaf 4.0.5, and here is the subset of dependent bundles:
30 | Active |  80 | 4.1.1.Final    | Netty/Codec
31 | Active |  80 | 4.1.1.Final    | Netty/Handler
32 | Active |  80 | 4.1.1.Final    | Netty/Resolver
34 | Active |  80 | 4.1.1.Final    | Netty/Common
35 | Active |  80 | 4.1.1.Final    | Netty/Transport
37 | Active |  80 | 4.1.1.Final    | Netty/Buffer
39 | Active |  80 | 1.1.33.Fork19  | Netty/TomcatNative [BoringSSL - Static]

BTW, the netty-tcnative(bundle 39) has the Bundle-NativeCode manifest entry added, according to the commit [ea27386](https://github.com/netty/netty-tcnative/commit/ea27386540f857032873292183f1c35e0c0cdbc6).

And the system information:
$ uname -a
Linux work.xsir 4.4.0-28-generic #47-Ubuntu SMP Fri Jun 24 10:09:13 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
$ java -version
java version "1.8.0_91"
Java(TM) SE Runtime Environment (build 1.8.0_91-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.91-b14, mixed mode)

I'm trying to debug the initialization of OpenSsl and netty-tcnative code. If I figured it out, I'll submit a PR ASAP.


Issue comments:
BTW, I have tried the 4.1.3.Final-SNAPSHOT version, it came out the same error too.

@xsir let us know... I have not much knowledge in terms of osgi.

I have figured it out finally. The root cause is: the `netty-tcnative` library is loaded by a ClassLoader that is fully independent of its bundle ClassLoader.

Specifically, the `netty-tcnative` is a bundle, which is loaded by OSGi framework with a separated ClassLoader. And the `netty-handler` is a bundle too, which contains the `OpenSsl` utility class. When the `OpenSsl` class is accessed, the static initialization will try to load the `netty-tcnative` library, which is embedded in `netty-tcnative` bundle. The OpenSsl loads the `tcnative` library through the `NativeLibraryLoader.loadFirstAvailable()` helper method, which will try to find the library in **PATH**, then in the JAR if failed. As we know, the library file is embedded in the bundle JAR, the `NativeLibraryLoader` will extract it to a tmp file, then call `System.load()` with the tmp file absolute path. The point is: in the SUN implementation, both `System.load()` and `System.loadLibrary()` are **CallerSensitive**, that means the native library is loaded by the caller class's ClassLoader! That's it, the `netty-tcnative` library is loaded by `NativeLibraryLoader.class`'s ClassLoader -- the netty-comon bundle's ClassLoader. But when the native method is called, it using the method definition class's ClassLoader, it can NOT find the native library in itself, then the `java.lang.UnsatisfiedLinkError: org.apache.tomcat.jni.Library.version(I)I` will be threw.

Yes, the root cause is the abused ClassLoader, it is discussed by @jjpp in [netty-tcnative issue 136](https://github.com/netty/netty-tcnative/issues/136). 

Then, the solution is clear: try to use the same ClassLoader to load the `netty-tcnative` library as the ClassLoader to initialize the native method. 

I hope I have made myself clear:) I'll submit a PR soon, thank you.

Fixed by https://github.com/netty/netty/pull/5545


Pull request review comments:
does this need to be public? can this just be `private static` and live in `NativeLibraryLoader`?

can we do the reflection initialization parts bits statically when initializing `NativeLibraryLoader`?

Hi, @Scottmitch ! That was a good idea. But, the helper class's duty is to delegate the calling to System.load() and it will be injected into the native library's ClassLoader, then when its method is invoked, the native library would be loaded into the native library's ClassLoader, NOT the netty-common's ClassLoader.
So, **the helper class should be as simple as possible, and should not be a inner class, otherwise we have to load the outer class and its dependencies**.
But, the helper class can be package.

The key point of this method is to load the helper class into the specified ClassLoader **if it not found in the ClassLoader**. Yes, the define class process is invoked only once if every thing is fine.

@xsir might be worth adding a java doc explaining this?

remove `/*package*/`

`return absolute ? System.load(libName) : System.loadLibrary(libname);`

`{@link System#load(String)}`, `{@link System#System.loadLibrary(String)}`

2016

put the `close()` calls into a `try { } catch (IOException ignore)` block so we not swallow other exceptions.

`{@link Class}` ... `{@link ClassLoader}` 

`IllegalStateException(...)` ?

@jasontedor can you comment about the impact of `SecurityManager` here ? Seems like you know this stuff in and out :)

is NOT

Use `{@link }`

We usually use `//` for comments.

IllegalStateException ?

Throwable ?

The System.load() will throw a UnsatisfiedLinkError if the library is not found. If we catch Throwable here, the System.load() will be called twice. That is why I catch Exception here. WDYT?

The System.load() is void too.

@xsir - Is it not possible / not worth it to move `Method defineClass` initialization here into a static context, and if so can you comment? It doesn't seem to depend upon any local state.

env -> environment?

To my view, the answer is not possible. Because the `NativeLibraryLoader` is a utility. Its method such as `load(String, ClassLoader)` is invoked with a **target** `ClassLoader` and we need to define the helper class into it, that is a dynamic process. For example, the `netty-tcnative` library is loaded by `OpenSsl` in `netty-handler` with the `SSL`'s `ClassLoader`, we have to inject the helper class into `SSL`'s `ClassLoader`. But for `netty-transport-native-epoll` library is loaded by `Native` with its `ClassLoader`, then we have to inject the helper class into `Native`'s `ClassLoader`.
The helper class injection process is nothing to do with `NativeLibraryLoader` itself . I have no idea how to make it static.

BTW, there are two native library involved in netty, both have the same OSGi loading issue. And there has another solution to this issue:
- `netty-tcnative` case, which provided a loading helper class `org.apache.tomcat.jni.Library`. Just like 9f2a2135, we should modify `OpenSsl`, instead of calling the `NativeLibraryLoader`, trying the helper class first (Depends on a properly `Bundle-NativeCode` manifest entry). _The helper class `Library` in `netty-tcnative` does NOT support loading library by absolute path._
- `netty-transport-native-epoll` case, we should modify `Native`, instead of calling the `NativeLibraryLoader`, trying the `System.load()` first (Depends on a properly `Bundle-NativeCode` manifest entry).

This solution is more simple, but there has a drawback: the library loading process is diff between OSGi and non-OSGi. In the OSGi, the library in `$PATH` has a higher priority than the one in the bundle. The non-OSGi is just the reverse. That's why I submitted a more complicated solution:)
Thanks for all of the comments!

thanks for the explanation ... fine as is

nit: consider restructuring to reduce the conditionals

``` java
int r;
while ((r = in.read(buf)) != -1) {
  out.write(buf, 0, r);
}
```

should we use a larger default size?

what I meant was using a bigger size for `ByteArrayOutputStream` (e.g `ByteArrayOutputStream(4096)`).

The security manager implications here are fine. 

@jasontedor so I understand you right that you think its fine to pull in in terms of `SecurityManager` stuff ? 


Patch that resolved the issue:
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 2f5a00836bb..9a1ba33cb2e 100644
--- a/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java
+++ b/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java
@@ -18,12 +18,16 @@
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.lang.reflect.Method;
 import java.net.URL;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.Locale;
 
@@ -186,7 +190,7 @@ public static void load(String name, ClassLoader loader) {
 
         if (url == null) {
             // Fall back to normal loading of JNI stuff
-            System.loadLibrary(name);
+            loadLibrary(loader, name, false);
             return;
         }
 
@@ -211,7 +215,7 @@ public static void load(String name, ClassLoader loader) {
             out.close();
             out = null;
 
-            System.load(tmpFile.getPath());
+            loadLibrary(loader, tmpFile.getPath(), true);
             loaded = true;
         } catch (Exception e) {
             throw (UnsatisfiedLinkError) new UnsatisfiedLinkError(
@@ -243,6 +247,117 @@ public static void load(String name, ClassLoader loader) {
         }
     }
 
+    /**
+     * Loading the native library into the specified {@link ClassLoader}.
+     * @param loader - The {@link ClassLoader} where the native library will be loaded into
+     * @param name - The native library path or name
+     * @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) {
+        try {
+            // Make sure the helper is belong to the target ClassLoader.
+            final Class<?> newHelper = tryToLoadClass(loader, NativeLibraryUtil.class);
+            loadLibraryByHelper(newHelper, name, absolute);
+        } catch (Exception e) { // Should by pass the UnsatisfiedLinkError here!
+            logger.debug("Unable to load the library: " + name + '.', e);
+            NativeLibraryUtil.loadLibrary(name, absolute);  // Fallback to local helper class.
+        }
+    }
+
+    private static void loadLibraryByHelper(final Class<?> helper, final String name, final boolean absolute) {
+        AccessController.doPrivileged(new PrivilegedAction<Object>() {
+            @Override
+            public Object run() {
+                try {
+                    // Invoke the helper to load the native library, if succeed, then the native
+                    // library belong to the specified ClassLoader.
+                    Method method = helper.getMethod("loadLibrary",
+                            new Class<?>[] { String.class, boolean.class });
+                    method.setAccessible(true);
+                    return method.invoke(null, name, absolute);
+                } catch (Exception e) {
+                    throw new IllegalStateException("Load library failed!", e);
+                }
+            }
+        });
+    }
+
+    /**
+     * Try to load the helper {@link Class} into specified {@link ClassLoader}.
+     * @param loader - The {@link ClassLoader} where to load the helper {@link Class}
+     * @param helper - The helper {@link Class}
+     * @return A new helper Class defined in the specified ClassLoader.
+     * @throws ClassNotFoundException Helper class not found or loading failed
+     */
+    private static Class<?> tryToLoadClass(final ClassLoader loader, final Class<?> helper)
+            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<Class<?>>() {
+                @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);
+                    }
+                }
+            });
+        }
+    }
+
+    /**
+     * Load the helper {@link Class} as a byte array, to be redefined in specified {@link ClassLoader}.
+     * @param clazz - The helper {@link Class} provided by this bundle
+     * @return The binary content of helper {@link Class}.
+     * @throws ClassNotFoundException Helper class not found or loading failed
+     */
+    private static byte[] classToByteArray(Class<?> clazz) throws ClassNotFoundException {
+        String fileName = clazz.getName();
+        int lastDot = fileName.lastIndexOf('.');
+        if (lastDot > 0) {
+            fileName = fileName.substring(lastDot + 1);
+        }
+        URL classUrl = clazz.getResource(fileName + ".class");
+        if (classUrl == null) {
+            throw new ClassNotFoundException(clazz.getName());
+        }
+        byte[] buf = new byte[1024];
+        ByteArrayOutputStream out = new ByteArrayOutputStream(4096);
+        InputStream in = null;
+        try {
+            in = classUrl.openStream();
+            for (int r; (r = in.read(buf)) != -1;) {
+                out.write(buf, 0, r);
+            }
+            return out.toByteArray();
+        } catch (IOException ex) {
+            throw new ClassNotFoundException(clazz.getName(), ex);
+        } finally {
+            closeQuietly(in);
+            closeQuietly(out);
+        }
+    }
+
+    private static void closeQuietly(java.io.Closeable c) {
+        if (c != null) {
+            try {
+                c.close();
+            } catch (IOException ignore) {
+                // ignore
+            }
+        }
+    }
+
     private NativeLibraryLoader() {
         // Utility
     }
diff --git a/common/src/main/java/io/netty/util/internal/NativeLibraryUtil.java b/common/src/main/java/io/netty/util/internal/NativeLibraryUtil.java
new file mode 100644
index 00000000000..1f9bf854574
--- /dev/null
+++ b/common/src/main/java/io/netty/util/internal/NativeLibraryUtil.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project licenses this file to you 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 io.netty.util.internal;
+
+/**
+ * A Utility to Call the {@link System#load(String)} or {@link System#loadLibrary(String)}.
+ * Because the {@link System#load(String)} and {@link System#loadLibrary(String)} are both
+ * CallerSensitive, it will load the native library into its caller's {@link ClassLoader}.
+ * In OSGi environment, we need this helper to delegate the calling to {@link System#load(String)}
+ * and it should be as simple as possible. It will be injected into the native library's
+ * ClassLoader when it is undefined. And therefore, when the defined new helper is invoked,
+ * the native library would be loaded into the native library's ClassLoader, not the
+ * caller's ClassLoader.
+ */
+final class NativeLibraryUtil {
+    /**
+     * Delegate the calling to {@link System#load(String)} or {@link System#loadLibrary(String)}.
+     * @param libName - The native library path or name
+     * @param absolute - Whether the native library will be loaded by path or by name
+     */
+    public static void loadLibrary(String libName, boolean absolute) {
+        if (absolute) {
+            System.load(libName);
+        } else {
+            System.loadLibrary(libName);
+        }
+    }
+
+    private NativeLibraryUtil() {
+        // Utility
+    }
+}


Test patch: