OSDN Git Service

Call JNI_OnUnload when class loaders get collected
authorMathieu Chartier <mathieuc@google.com>
Wed, 23 Sep 2015 21:52:39 +0000 (14:52 -0700)
committerMathieu Chartier <mathieuc@google.com>
Fri, 25 Sep 2015 18:05:00 +0000 (11:05 -0700)
Added test case to 141-class-unload.

Bug: 22720414
Change-Id: I0575fae72521520a17587e8b0088bf8112705ad8

27 files changed:
runtime/gc/heap.cc
runtime/java_vm_ext.cc
runtime/java_vm_ext.h
test/004-JniTest/expected.txt
test/004-JniTest/jni_test.cc
test/004-ReferenceMap/expected.txt
test/004-SignalTest/expected.txt
test/004-StackWalk/expected.txt
test/004-UnsafeTest/expected.txt
test/044-proxy/expected.txt
test/051-thread/expected.txt
test/088-monitor-verification/expected.txt
test/115-native-bridge/expected.txt
test/117-nopatchoat/expected.txt
test/119-noimage-patchoat/expected.txt
test/137-cfi/expected.txt
test/139-register-natives/expected.txt
test/141-class-unload/expected.txt
test/141-class-unload/jni_unload.cc [new file with mode: 0644]
test/141-class-unload/src-ex/IntHolder.java
test/141-class-unload/src/Main.java
test/454-get-vreg/expected.txt
test/455-set-vreg/expected.txt
test/461-get-reference-vreg/expected.txt
test/466-get-live-vreg/expected.txt
test/497-inlining-and-class-loader/expected.txt
test/Android.libarttest.mk

index cfe7713..7d664fa 100644 (file)
@@ -1963,6 +1963,10 @@ HomogeneousSpaceCompactResult Heap::PerformHomogeneousSpaceCompact() {
   GrowForUtilization(semi_space_collector_);
   LogGC(kGcCauseHomogeneousSpaceCompact, collector);
   FinishGC(self, collector::kGcTypeFull);
+  {
+    ScopedObjectAccess soa(self);
+    soa.Vm()->UnloadNativeLibraries();
+  }
   return HomogeneousSpaceCompactResult::kSuccess;
 }
 
@@ -2104,6 +2108,10 @@ void Heap::TransitionCollector(CollectorType collector_type) {
   DCHECK(collector != nullptr);
   LogGC(kGcCauseCollectorTransition, collector);
   FinishGC(self, collector::kGcTypeFull);
+  {
+    ScopedObjectAccess soa(self);
+    soa.Vm()->UnloadNativeLibraries();
+  }
   int32_t after_allocated = num_bytes_allocated_.LoadSequentiallyConsistent();
   int32_t delta_allocated = before_allocated - after_allocated;
   std::string saved_str;
@@ -2588,6 +2596,12 @@ collector::GcType Heap::CollectGarbageInternal(collector::GcType gc_type,
   FinishGC(self, gc_type);
   // Inform DDMS that a GC completed.
   Dbg::GcDidFinish();
+  // Unload native libraries for class unloading. We do this after calling FinishGC to prevent
+  // deadlocks in case the JNI_OnUnload function does allocations.
+  {
+    ScopedObjectAccess soa(self);
+    soa.Vm()->UnloadNativeLibraries();
+  }
   return gc_type;
 }
 
index 531e039..ab70d02 100644 (file)
@@ -60,7 +60,7 @@ class SharedLibrary {
       : path_(path),
         handle_(handle),
         needs_native_bridge_(false),
-        class_loader_(env->NewGlobalRef(class_loader)),
+        class_loader_(env->NewWeakGlobalRef(class_loader)),
         jni_on_load_lock_("JNI_OnLoad lock"),
         jni_on_load_cond_("JNI_OnLoad condition variable", jni_on_load_lock_),
         jni_on_load_thread_id_(self->GetThreadId()),
@@ -70,11 +70,11 @@ class SharedLibrary {
   ~SharedLibrary() {
     Thread* self = Thread::Current();
     if (self != nullptr) {
-      self->GetJniEnv()->DeleteGlobalRef(class_loader_);
+      self->GetJniEnv()->DeleteWeakGlobalRef(class_loader_);
     }
   }
 
-  jobject GetClassLoader() const {
+  jweak GetClassLoader() const {
     return class_loader_;
   }
 
@@ -131,7 +131,13 @@ class SharedLibrary {
     return needs_native_bridge_;
   }
 
-  void* FindSymbol(const std::string& symbol_name) {
+  void* FindSymbol(const std::string& symbol_name, const char* shorty = nullptr) {
+    return NeedsNativeBridge()
+        ? FindSymbolWithNativeBridge(symbol_name.c_str(), shorty)
+        : FindSymbolWithoutNativeBridge(symbol_name.c_str());
+  }
+
+  void* FindSymbolWithoutNativeBridge(const std::string& symbol_name) {
     CHECK(!NeedsNativeBridge());
 
     return dlsym(handle_, symbol_name.c_str());
@@ -160,9 +166,9 @@ class SharedLibrary {
   // True if a native bridge is required.
   bool needs_native_bridge_;
 
-  // The ClassLoader this library is associated with, a global JNI reference that is
+  // The ClassLoader this library is associated with, a weak global JNI reference that is
   // created/deleted with the scope of the library.
-  const jobject class_loader_;
+  const jweak class_loader_;
 
   // Guards remaining items.
   Mutex jni_on_load_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
@@ -184,7 +190,10 @@ class Libraries {
     STLDeleteValues(&libraries_);
   }
 
-  void Dump(std::ostream& os) const {
+  // NO_THREAD_SAFETY_ANALYSIS since this may be called from Dumpable. Dumpable can't be annotated
+  // properly due to the template. The caller should be holding the jni_libraries_lock_.
+  void Dump(std::ostream& os) const NO_THREAD_SAFETY_ANALYSIS {
+    Locks::jni_libraries_lock_->AssertHeld(Thread::Current());
     bool first = true;
     for (const auto& library : libraries_) {
       if (!first) {
@@ -195,16 +204,17 @@ class Libraries {
     }
   }
 
-  size_t size() const {
+  size_t size() const REQUIRES(Locks::jni_libraries_lock_) {
     return libraries_.size();
   }
 
-  SharedLibrary* Get(const std::string& path) {
+  SharedLibrary* Get(const std::string& path) REQUIRES(Locks::jni_libraries_lock_) {
     auto it = libraries_.find(path);
     return (it == libraries_.end()) ? nullptr : it->second;
   }
 
-  void Put(const std::string& path, SharedLibrary* library) {
+  void Put(const std::string& path, SharedLibrary* library)
+      REQUIRES(Locks::jni_libraries_lock_) {
     libraries_.Put(path, library);
   }
 
@@ -217,24 +227,18 @@ class Libraries {
     const mirror::ClassLoader* declaring_class_loader = m->GetDeclaringClass()->GetClassLoader();
     ScopedObjectAccessUnchecked soa(Thread::Current());
     for (const auto& lib : libraries_) {
-      SharedLibrary* library = lib.second;
+      SharedLibrary* const library = lib.second;
       if (soa.Decode<mirror::ClassLoader*>(library->GetClassLoader()) != declaring_class_loader) {
         // We only search libraries loaded by the appropriate ClassLoader.
         continue;
       }
       // Try the short name then the long name...
-      void* fn;
-      if (library->NeedsNativeBridge()) {
-        const char* shorty = m->GetShorty();
-        fn = library->FindSymbolWithNativeBridge(jni_short_name, shorty);
-        if (fn == nullptr) {
-          fn = library->FindSymbolWithNativeBridge(jni_long_name, shorty);
-        }
-      } else {
-        fn = library->FindSymbol(jni_short_name);
-        if (fn == nullptr) {
-          fn = library->FindSymbol(jni_long_name);
-        }
+      const char* shorty = library->NeedsNativeBridge()
+          ? m->GetShorty()
+          : nullptr;
+      void* fn = library->FindSymbol(jni_short_name, shorty);
+      if (fn == nullptr) {
+        fn = library->FindSymbol(jni_long_name, shorty);
       }
       if (fn != nullptr) {
         VLOG(jni) << "[Found native code for " << PrettyMethod(m)
@@ -249,11 +253,46 @@ class Libraries {
     return nullptr;
   }
 
+  // Unload native libraries with cleared class loaders.
+  void UnloadNativeLibraries()
+      REQUIRES(!Locks::jni_libraries_lock_)
+      SHARED_REQUIRES(Locks::mutator_lock_) {
+    ScopedObjectAccessUnchecked soa(Thread::Current());
+    typedef void (*JNI_OnUnloadFn)(JavaVM*, void*);
+    std::vector<JNI_OnUnloadFn> unload_functions;
+    {
+      MutexLock mu(soa.Self(), *Locks::jni_libraries_lock_);
+      for (auto it = libraries_.begin(); it != libraries_.end(); ) {
+        SharedLibrary* const library = it->second;
+        // If class loader is null then it was unloaded, call JNI_OnUnload.
+        if (soa.Decode<mirror::ClassLoader*>(library->GetClassLoader()) == nullptr) {
+          void* const sym = library->FindSymbol("JNI_OnUnload", nullptr);
+          if (sym == nullptr) {
+            VLOG(jni) << "[No JNI_OnUnload found in \"" << library->GetPath() << "\"]";
+          } else {
+            VLOG(jni) << "[JNI_OnUnload found for \"" << library->GetPath() << "\"]";
+            JNI_OnUnloadFn jni_on_unload = reinterpret_cast<JNI_OnUnloadFn>(sym);
+            unload_functions.push_back(jni_on_unload);
+          }
+          delete library;
+          it = libraries_.erase(it);
+        } else {
+          ++it;
+        }
+      }
+    }
+    // Do this without holding the jni libraries lock to prevent possible deadlocks.
+    for (JNI_OnUnloadFn fn : unload_functions) {
+      VLOG(jni) << "Calling JNI_OnUnload";
+      (*fn)(soa.Vm(), nullptr);
+    }
+  }
+
  private:
-  AllocationTrackingSafeMap<std::string, SharedLibrary*, kAllocatorTagJNILibraries> libraries_;
+  AllocationTrackingSafeMap<std::string, SharedLibrary*, kAllocatorTagJNILibraries> libraries_
+      GUARDED_BY(Locks::jni_libraries_lock_);
 };
 
-
 class JII {
  public:
   static jint DestroyJavaVM(JavaVM* vm) {
@@ -641,6 +680,10 @@ void JavaVMExt::DumpReferenceTables(std::ostream& os) {
   }
 }
 
+void JavaVMExt::UnloadNativeLibraries() {
+  libraries_.get()->UnloadNativeLibraries();
+}
+
 bool JavaVMExt::LoadNativeLibrary(JNIEnv* env, const std::string& path, jobject class_loader,
                                   std::string* error_msg) {
   error_msg->clear();
@@ -738,10 +781,8 @@ bool JavaVMExt::LoadNativeLibrary(JNIEnv* env, const std::string& path, jobject
   void* sym;
   if (needs_native_bridge) {
     library->SetNeedsNativeBridge();
-    sym = library->FindSymbolWithNativeBridge("JNI_OnLoad", nullptr);
-  } else {
-    sym = dlsym(handle, "JNI_OnLoad");
   }
+  sym = library->FindSymbol("JNI_OnLoad", nullptr);
   if (sym == nullptr) {
     VLOG(jni) << "[No JNI_OnLoad found in \"" << path << "\"]";
     was_successful = true;
index b539bbd..c1fbdc0 100644 (file)
@@ -88,6 +88,11 @@ class JavaVMExt : public JavaVM {
   bool LoadNativeLibrary(JNIEnv* env, const std::string& path, jobject javaLoader,
                          std::string* error_msg);
 
+  // Unload native libraries with cleared class loaders.
+  void UnloadNativeLibraries()
+      REQUIRES(!Locks::jni_libraries_lock_)
+      SHARED_REQUIRES(Locks::mutator_lock_);
+
   /**
    * Returns a pointer to the code for the native method 'm', found
    * using dlsym(3) on every native library that's been loaded so far.
@@ -184,7 +189,9 @@ class JavaVMExt : public JavaVM {
   // Not guarded by globals_lock since we sometimes use SynchronizedGet in Thread::DecodeJObject.
   IndirectReferenceTable globals_;
 
-  std::unique_ptr<Libraries> libraries_ GUARDED_BY(Locks::jni_libraries_lock_);
+  // No lock annotation since UnloadNativeLibraries is called on libraries_ but locks the
+  // jni_libraries_lock_ internally.
+  std::unique_ptr<Libraries> libraries_;
 
   // Used by -Xcheck:jni.
   const JNIInvokeInterface* const unchecked_functions_;
index 49d9cc0..86ab37e 100644 (file)
@@ -1,3 +1,4 @@
+JNI_OnLoad called
 Super.<init>
 Super.<init>
 Subclass.<init>
index db0dd32..370bd6a 100644 (file)
@@ -15,8 +15,9 @@
  */
 
 #include <assert.h>
-#include <stdio.h>
+#include <iostream>
 #include <pthread.h>
+#include <stdio.h>
 #include <vector>
 
 #include "jni.h"
@@ -31,6 +32,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM *vm, void *) {
   assert(vm != nullptr);
   assert(jvm == nullptr);
   jvm = vm;
+  std::cout << "JNI_OnLoad called" << std::endl;
   return JNI_VERSION_1_6;
 }
 
index fd5ec00..b3a0e1c 100644 (file)
@@ -1,3 +1,4 @@
+JNI_OnLoad called
 init signal test
 Caught NullPointerException
 Caught StackOverflowError
index bde0024..5af68cd 100644 (file)
@@ -1,3 +1,4 @@
+JNI_OnLoad called
 1st call
 172001234567891011121314151617181920652310201919
 2nd call
index f86948a..052c8fa 100644 (file)
@@ -93,4 +93,5 @@ Invocation of public abstract java.lang.String NarrowingTest$I2.foo()
 Got expected exception
 Proxy narrowed invocation return type passed
 5.8
+JNI_OnLoad called
 callback
index 54e34af..c6cd4f8 100644 (file)
@@ -1,3 +1,4 @@
+JNI_OnLoad called
 thread test starting
 testThreadCapacity thread count: 512
 testThreadDaemons starting thread 'TestDaemonThread'
index 13b8c73..f252f6f 100644 (file)
@@ -1,3 +1,4 @@
+JNI_OnLoad called
 recursiveSync ok
 nestedMayThrow ok
 constantLock ok
index 372ecd0..b003307 100644 (file)
@@ -17,6 +17,7 @@ Test ART callbacks: all JNI function number is 11.
     name:testSignal, signature:()I, shorty:I.
     name:testZeroLengthByteBuffers, signature:()V, shorty:V.
 trampoline_JNI_OnLoad called!
+JNI_OnLoad called
 Getting trampoline for Java_Main_testFindClassOnAttachedNativeThread with shorty V.
 trampoline_Java_Main_testFindClassOnAttachedNativeThread called!
 Getting trampoline for Java_Main_testFindFieldOnAttachedNativeThreadNative with shorty V.
index 5cc02d1..0cd4715 100644 (file)
@@ -1,9 +1,12 @@
 Run without dex2oat/patchoat
+JNI_OnLoad called
 dex2oat & patchoat are disabled, has oat is true, has executable oat is expected.
 This is a function call
 Run with dexoat/patchoat
+JNI_OnLoad called
 dex2oat & patchoat are enabled, has oat is true, has executable oat is expected.
 This is a function call
 Run default
+JNI_OnLoad called
 dex2oat & patchoat are enabled, has oat is true, has executable oat is expected.
 This is a function call
index ed13662..9b9db58 100644 (file)
@@ -1,8 +1,11 @@
 Run -Xnoimage-dex2oat -Xpatchoat:/system/bin/false
+JNI_OnLoad called
 Has image is false, is image dex2oat enabled is false.
 Run -Xnoimage-dex2oat -Xpatchoat:/system/bin/false -Xno-dex-file-fallback
 Failed to initialize runtime (check log for details)
 Run -Ximage-dex2oat
+JNI_OnLoad called
 Has image is true, is image dex2oat enabled is true.
 Run default
+JNI_OnLoad called
 Has image is true, is image dex2oat enabled is true.
index be2671e..124398f 100644 (file)
@@ -7,3 +7,6 @@ null
 null
 loader null false
 loader null false
+JNI_OnLoad called
+JNI_OnUnload called
+null
diff --git a/test/141-class-unload/jni_unload.cc b/test/141-class-unload/jni_unload.cc
new file mode 100644 (file)
index 0000000..894f28c
--- /dev/null
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed 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.
+ */
+
+#include "jni.h"
+
+#include <iostream>
+
+extern "C" JNIEXPORT void JNI_OnUnload(JavaVM*, void *) {
+  // std::cout since LOG(INFO) adds extra stuff like pid.
+  std::cout << "JNI_OnUnload called" << std::endl;
+}
index 0a1c1e6..b4651af 100644 (file)
@@ -30,4 +30,8 @@ public class IntHolder {
     public static void runGC() {
         Runtime.getRuntime().gc();
     }
+
+    public static void loadLibrary(String name) {
+        System.loadLibrary(name);
+    }
 }
index 3a2ac9b..da15746 100644 (file)
@@ -20,8 +20,10 @@ import java.lang.reflect.Method;
 
 public class Main {
     static final String DEX_FILE = System.getenv("DEX_LOCATION") + "/141-class-unload-ex.jar";
+    static String nativeLibraryName;
 
     public static void main(String[] args) throws Exception {
+        nativeLibraryName = args[0];
         Class pathClassLoader = Class.forName("dalvik.system.PathClassLoader");
         if (pathClassLoader == null) {
             throw new AssertionError("Couldn't find path class loader class");
@@ -34,6 +36,8 @@ public class Main {
             testNoUnloadInvoke(constructor);
             // Test that we don't unload if we have an instance.
             testNoUnloadInstance(constructor);
+            // Test JNI_OnLoad and JNI_OnUnload.
+            testLoadAndUnloadLibrary(constructor);
             // Stress test to make sure we dont leak memory.
             stressTest(constructor);
         } catch (Exception e) {
@@ -63,6 +67,14 @@ public class Main {
         System.out.println(loader.get());
     }
 
+    private static void testLoadAndUnloadLibrary(Constructor constructor) throws Exception {
+        WeakReference<ClassLoader> loader = setUpLoadLibrary(constructor);
+        // No strong refernces to class loader, should get unloaded.
+        Runtime.getRuntime().gc();
+        // If the weak reference is cleared, then it was unloaded.
+        System.out.println(loader.get());
+    }
+
     private static void testNoUnloadInvoke(Constructor constructor) throws Exception {
         WeakReference<ClassLoader> loader =
             new WeakReference((ClassLoader) constructor.newInstance(
@@ -109,4 +121,13 @@ public class Main {
         return new WeakReference(loader);
     }
 
+    private static WeakReference<ClassLoader> setUpLoadLibrary(Constructor constructor)
+        throws Exception {
+        ClassLoader loader = (ClassLoader) constructor.newInstance(
+            DEX_FILE, ClassLoader.getSystemClassLoader());
+        Class intHolder = loader.loadClass("IntHolder");
+        Method setValue = intHolder.getDeclaredMethod("loadLibrary", String.class);
+        setValue.invoke(intHolder, nativeLibraryName);
+        return new WeakReference(loader);
+    }
 }
index f5b9fe0..905dbfd 100644 (file)
@@ -1,3 +1,4 @@
+JNI_OnLoad called
 java.lang.Exception
        at Main.$noinline$bar(Main.java:124)
        at Level2.$inline$bar(Level1.java:25)
index 7f05a04..e43ea90 100644 (file)
@@ -33,6 +33,7 @@ LIBARTTEST_COMMON_SRC_FILES := \
   1337-gc-coverage/gc_coverage.cc \
   137-cfi/cfi.cc \
   139-register-natives/regnative.cc \
+  141-class-unload/jni_unload.cc \
   454-get-vreg/get_vreg_jni.cc \
   455-set-vreg/set_vreg_jni.cc \
   457-regs/regs_jni.cc \