Call JNI_OnUnload when class loaders get collected
Added test case to 141-class-unload.
Bug: 22720414
Change-Id: I0575fae72521520a17587e8b0088bf8112705ad8
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index cfe7713..7d664fa 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1963,6 +1963,10 @@
GrowForUtilization(semi_space_collector_);
LogGC(kGcCauseHomogeneousSpaceCompact, collector);
FinishGC(self, collector::kGcTypeFull);
+ {
+ ScopedObjectAccess soa(self);
+ soa.Vm()->UnloadNativeLibraries();
+ }
return HomogeneousSpaceCompactResult::kSuccess;
}
@@ -2104,6 +2108,10 @@
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 @@
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;
}
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index 531e039..ab70d02 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -60,7 +60,7 @@
: 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 @@
~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 @@
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 @@
// 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 @@
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 @@
}
}
- 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 @@
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,10 +253,45 @@
return nullptr;
}
- private:
- AllocationTrackingSafeMap<std::string, SharedLibrary*, kAllocatorTagJNILibraries> libraries_;
-};
+ // 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_
+ GUARDED_BY(Locks::jni_libraries_lock_);
+};
class JII {
public:
@@ -641,6 +680,10 @@
}
}
+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 @@
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;
diff --git a/runtime/java_vm_ext.h b/runtime/java_vm_ext.h
index b539bbd..c1fbdc0 100644
--- a/runtime/java_vm_ext.h
+++ b/runtime/java_vm_ext.h
@@ -88,6 +88,11 @@
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 @@
// 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_;
diff --git a/test/004-JniTest/expected.txt b/test/004-JniTest/expected.txt
index 49d9cc0..86ab37e 100644
--- a/test/004-JniTest/expected.txt
+++ b/test/004-JniTest/expected.txt
@@ -1,3 +1,4 @@
+JNI_OnLoad called
Super.<init>
Super.<init>
Subclass.<init>
diff --git a/test/004-JniTest/jni_test.cc b/test/004-JniTest/jni_test.cc
index db0dd32..370bd6a 100644
--- a/test/004-JniTest/jni_test.cc
+++ b/test/004-JniTest/jni_test.cc
@@ -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 @@
assert(vm != nullptr);
assert(jvm == nullptr);
jvm = vm;
+ std::cout << "JNI_OnLoad called" << std::endl;
return JNI_VERSION_1_6;
}
diff --git a/test/004-ReferenceMap/expected.txt b/test/004-ReferenceMap/expected.txt
index e69de29..6a5618e 100644
--- a/test/004-ReferenceMap/expected.txt
+++ b/test/004-ReferenceMap/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/004-SignalTest/expected.txt b/test/004-SignalTest/expected.txt
index fd5ec00..b3a0e1c 100644
--- a/test/004-SignalTest/expected.txt
+++ b/test/004-SignalTest/expected.txt
@@ -1,3 +1,4 @@
+JNI_OnLoad called
init signal test
Caught NullPointerException
Caught StackOverflowError
diff --git a/test/004-StackWalk/expected.txt b/test/004-StackWalk/expected.txt
index bde0024..5af68cd 100644
--- a/test/004-StackWalk/expected.txt
+++ b/test/004-StackWalk/expected.txt
@@ -1,3 +1,4 @@
+JNI_OnLoad called
1st call
172001234567891011121314151617181920652310201919
2nd call
diff --git a/test/004-UnsafeTest/expected.txt b/test/004-UnsafeTest/expected.txt
index e69de29..6a5618e 100644
--- a/test/004-UnsafeTest/expected.txt
+++ b/test/004-UnsafeTest/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/044-proxy/expected.txt b/test/044-proxy/expected.txt
index f86948a..052c8fa 100644
--- a/test/044-proxy/expected.txt
+++ b/test/044-proxy/expected.txt
@@ -93,4 +93,5 @@
Got expected exception
Proxy narrowed invocation return type passed
5.8
+JNI_OnLoad called
callback
diff --git a/test/051-thread/expected.txt b/test/051-thread/expected.txt
index 54e34af..c6cd4f8 100644
--- a/test/051-thread/expected.txt
+++ b/test/051-thread/expected.txt
@@ -1,3 +1,4 @@
+JNI_OnLoad called
thread test starting
testThreadCapacity thread count: 512
testThreadDaemons starting thread 'TestDaemonThread'
diff --git a/test/088-monitor-verification/expected.txt b/test/088-monitor-verification/expected.txt
index 13b8c73..f252f6f 100644
--- a/test/088-monitor-verification/expected.txt
+++ b/test/088-monitor-verification/expected.txt
@@ -1,3 +1,4 @@
+JNI_OnLoad called
recursiveSync ok
nestedMayThrow ok
constantLock ok
diff --git a/test/115-native-bridge/expected.txt b/test/115-native-bridge/expected.txt
index 372ecd0..b003307 100644
--- a/test/115-native-bridge/expected.txt
+++ b/test/115-native-bridge/expected.txt
@@ -17,6 +17,7 @@
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.
diff --git a/test/117-nopatchoat/expected.txt b/test/117-nopatchoat/expected.txt
index 5cc02d1..0cd4715 100644
--- a/test/117-nopatchoat/expected.txt
+++ b/test/117-nopatchoat/expected.txt
@@ -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
diff --git a/test/119-noimage-patchoat/expected.txt b/test/119-noimage-patchoat/expected.txt
index ed13662..9b9db58 100644
--- a/test/119-noimage-patchoat/expected.txt
+++ b/test/119-noimage-patchoat/expected.txt
@@ -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.
diff --git a/test/137-cfi/expected.txt b/test/137-cfi/expected.txt
index e69de29..6a5618e 100644
--- a/test/137-cfi/expected.txt
+++ b/test/137-cfi/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/139-register-natives/expected.txt b/test/139-register-natives/expected.txt
index e69de29..6a5618e 100644
--- a/test/139-register-natives/expected.txt
+++ b/test/139-register-natives/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/141-class-unload/expected.txt b/test/141-class-unload/expected.txt
index be2671e..124398f 100644
--- a/test/141-class-unload/expected.txt
+++ b/test/141-class-unload/expected.txt
@@ -7,3 +7,6 @@
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
index 0000000..894f28c
--- /dev/null
+++ b/test/141-class-unload/jni_unload.cc
@@ -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;
+}
diff --git a/test/141-class-unload/src-ex/IntHolder.java b/test/141-class-unload/src-ex/IntHolder.java
index 0a1c1e6..b4651af 100644
--- a/test/141-class-unload/src-ex/IntHolder.java
+++ b/test/141-class-unload/src-ex/IntHolder.java
@@ -30,4 +30,8 @@
public static void runGC() {
Runtime.getRuntime().gc();
}
+
+ public static void loadLibrary(String name) {
+ System.loadLibrary(name);
+ }
}
diff --git a/test/141-class-unload/src/Main.java b/test/141-class-unload/src/Main.java
index 3a2ac9b..da15746 100644
--- a/test/141-class-unload/src/Main.java
+++ b/test/141-class-unload/src/Main.java
@@ -20,8 +20,10 @@
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 @@
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 @@
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 @@
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);
+ }
}
diff --git a/test/454-get-vreg/expected.txt b/test/454-get-vreg/expected.txt
index e69de29..6a5618e 100644
--- a/test/454-get-vreg/expected.txt
+++ b/test/454-get-vreg/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/455-set-vreg/expected.txt b/test/455-set-vreg/expected.txt
index e69de29..6a5618e 100644
--- a/test/455-set-vreg/expected.txt
+++ b/test/455-set-vreg/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/461-get-reference-vreg/expected.txt b/test/461-get-reference-vreg/expected.txt
index e69de29..6a5618e 100644
--- a/test/461-get-reference-vreg/expected.txt
+++ b/test/461-get-reference-vreg/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/466-get-live-vreg/expected.txt b/test/466-get-live-vreg/expected.txt
index e69de29..6a5618e 100644
--- a/test/466-get-live-vreg/expected.txt
+++ b/test/466-get-live-vreg/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/497-inlining-and-class-loader/expected.txt b/test/497-inlining-and-class-loader/expected.txt
index f5b9fe0..905dbfd 100644
--- a/test/497-inlining-and-class-loader/expected.txt
+++ b/test/497-inlining-and-class-loader/expected.txt
@@ -1,3 +1,4 @@
+JNI_OnLoad called
java.lang.Exception
at Main.$noinline$bar(Main.java:124)
at Level2.$inline$bar(Level1.java:25)
diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk
index 7f05a04..e43ea90 100644
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -33,6 +33,7 @@
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 \