Keep internal stack trace methods declaring classes live
We now store the declaring classes in the internal stack trace to
make sure class unloading doesn't unload any classes owning methods
in the stack trace.
This fixes DexClassLoaderTest in libcore. Added regression test.
Bug: 22720414
Change-Id: I185f87c8ec0807e83f4661bd5bb5652dba6fc281
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 82e6fb0..65f71ef 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1966,15 +1966,32 @@
pointer_size_(Runtime::Current()->GetClassLinker()->GetImagePointerSize()) {}
bool Init(int depth) SHARED_REQUIRES(Locks::mutator_lock_) ACQUIRE(Roles::uninterruptible_) {
- // Allocate method trace with format [method pointers][pcs].
- auto* cl = Runtime::Current()->GetClassLinker();
- trace_ = cl->AllocPointerArray(self_, depth * 2);
- const char* last_no_suspend_cause =
- self_->StartAssertNoThreadSuspension("Building internal stack trace");
- if (trace_ == nullptr) {
+ // Allocate method trace as an object array where the first element is a pointer array that
+ // contains the ArtMethod pointers and dex PCs. The rest of the elements are the declaring
+ // class of the ArtMethod pointers.
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ StackHandleScope<1> hs(self_);
+ mirror::Class* array_class = class_linker->GetClassRoot(ClassLinker::kObjectArrayClass);
+ // The first element is the methods and dex pc array, the other elements are declaring classes
+ // for the methods to ensure classes in the stack trace don't get unloaded.
+ Handle<mirror::ObjectArray<mirror::Object>> trace(
+ hs.NewHandle(
+ mirror::ObjectArray<mirror::Object>::Alloc(hs.Self(), array_class, depth + 1)));
+ if (trace.Get() == nullptr) {
+ // Acquire uninterruptible_ in all paths.
+ self_->StartAssertNoThreadSuspension("Building internal stack trace");
self_->AssertPendingOOMException();
return false;
}
+ mirror::PointerArray* methods_and_pcs = class_linker->AllocPointerArray(self_, depth * 2);
+ const char* last_no_suspend_cause =
+ self_->StartAssertNoThreadSuspension("Building internal stack trace");
+ if (methods_and_pcs == nullptr) {
+ self_->AssertPendingOOMException();
+ return false;
+ }
+ trace->Set(0, methods_and_pcs);
+ trace_ = trace.Get();
// If We are called from native, use non-transactional mode.
CHECK(last_no_suspend_cause == nullptr) << last_no_suspend_cause;
return true;
@@ -1996,16 +2013,24 @@
if (m->IsRuntimeMethod()) {
return true; // Ignore runtime frames (in particular callee save).
}
- trace_->SetElementPtrSize<kTransactionActive>(
- count_, m, pointer_size_);
- trace_->SetElementPtrSize<kTransactionActive>(
- trace_->GetLength() / 2 + count_, m->IsProxyMethod() ? DexFile::kDexNoIndex : GetDexPc(),
- pointer_size_);
+ mirror::PointerArray* trace_methods_and_pcs = GetTraceMethodsAndPCs();
+ trace_methods_and_pcs->SetElementPtrSize<kTransactionActive>(count_, m, pointer_size_);
+ trace_methods_and_pcs->SetElementPtrSize<kTransactionActive>(
+ trace_methods_and_pcs->GetLength() / 2 + count_,
+ m->IsProxyMethod() ? DexFile::kDexNoIndex : GetDexPc(),
+ pointer_size_);
+ // Save the declaring class of the method to ensure that the declaring classes of the methods
+ // do not get unloaded while the stack trace is live.
+ trace_->Set(count_ + 1, m->GetDeclaringClass());
++count_;
return true;
}
- mirror::PointerArray* GetInternalStackTrace() const {
+ mirror::PointerArray* GetTraceMethodsAndPCs() const SHARED_REQUIRES(Locks::mutator_lock_) {
+ return down_cast<mirror::PointerArray*>(trace_->Get(0));
+ }
+
+ mirror::ObjectArray<mirror::Object>* GetInternalStackTrace() const {
return trace_;
}
@@ -2015,8 +2040,11 @@
int32_t skip_depth_;
// Current position down stack trace.
uint32_t count_;
- // An array of the methods on the stack, the last entries are the dex PCs.
- mirror::PointerArray* trace_;
+ // An object array where the first element is a pointer array that contains the ArtMethod
+ // pointers on the stack and dex PCs. The rest of the elements are the declaring
+ // class of the ArtMethod pointers. trace_[i+1] contains the declaring class of the ArtMethod of
+ // the i'th frame.
+ mirror::ObjectArray<mirror::Object>* trace_;
// For cross compilation.
const size_t pointer_size_;
@@ -2039,11 +2067,12 @@
return nullptr; // Allocation failed.
}
build_trace_visitor.WalkStack();
- mirror::PointerArray* trace = build_trace_visitor.GetInternalStackTrace();
+ mirror::ObjectArray<mirror::Object>* trace = build_trace_visitor.GetInternalStackTrace();
if (kIsDebugBuild) {
- // Second half is dex PCs.
- for (uint32_t i = 0; i < static_cast<uint32_t>(trace->GetLength() / 2); ++i) {
- auto* method = trace->GetElementPtrSize<ArtMethod*>(
+ mirror::PointerArray* trace_methods = build_trace_visitor.GetTraceMethodsAndPCs();
+ // Second half of trace_methods is dex PCs.
+ for (uint32_t i = 0; i < static_cast<uint32_t>(trace_methods->GetLength() / 2); ++i) {
+ auto* method = trace_methods->GetElementPtrSize<ArtMethod*>(
i, Runtime::Current()->GetClassLinker()->GetImagePointerSize());
CHECK(method != nullptr);
}
@@ -2062,12 +2091,16 @@
}
jobjectArray Thread::InternalStackTraceToStackTraceElementArray(
- const ScopedObjectAccessAlreadyRunnable& soa, jobject internal, jobjectArray output_array,
+ const ScopedObjectAccessAlreadyRunnable& soa,
+ jobject internal,
+ jobjectArray output_array,
int* stack_depth) {
- // Decode the internal stack trace into the depth, method trace and PC trace
- int32_t depth = soa.Decode<mirror::PointerArray*>(internal)->GetLength() / 2;
+ // Decode the internal stack trace into the depth, method trace and PC trace.
+ // Subtract one for the methods and PC trace.
+ int32_t depth = soa.Decode<mirror::Array*>(internal)->GetLength() - 1;
+ DCHECK_GE(depth, 0);
- auto* cl = Runtime::Current()->GetClassLinker();
+ ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
jobjectArray result;
@@ -2081,7 +2114,7 @@
} else {
// Create java_trace array and place in local reference table
mirror::ObjectArray<mirror::StackTraceElement>* java_traces =
- cl->AllocStackTraceElementArray(soa.Self(), depth);
+ class_linker->AllocStackTraceElementArray(soa.Self(), depth);
if (java_traces == nullptr) {
return nullptr;
}
@@ -2093,7 +2126,12 @@
}
for (int32_t i = 0; i < depth; ++i) {
- auto* method_trace = soa.Decode<mirror::PointerArray*>(internal);
+ mirror::ObjectArray<mirror::Object>* decoded_traces =
+ soa.Decode<mirror::Object*>(internal)->AsObjectArray<mirror::Object>();
+ // Methods and dex PC trace is element 0.
+ DCHECK(decoded_traces->Get(0)->IsIntArray() || decoded_traces->Get(0)->IsLongArray());
+ mirror::PointerArray* const method_trace =
+ down_cast<mirror::PointerArray*>(decoded_traces->Get(0));
// Prepare parameters for StackTraceElement(String cls, String method, String file, int line)
ArtMethod* method = method_trace->GetElementPtrSize<ArtMethod*>(i, sizeof(void*));
uint32_t dex_pc = method_trace->GetElementPtrSize<uint32_t>(
diff --git a/test/141-class-unload/expected.txt b/test/141-class-unload/expected.txt
index ff65a70..53d7abe 100644
--- a/test/141-class-unload/expected.txt
+++ b/test/141-class-unload/expected.txt
@@ -16,3 +16,8 @@
JNI_OnLoad called
JNI_OnUnload called
null
+1
+2
+JNI_OnLoad called
+class null false test
+JNI_OnUnload called
diff --git a/test/141-class-unload/src-ex/IntHolder.java b/test/141-class-unload/src-ex/IntHolder.java
index e4aa6b8..feff0d2 100644
--- a/test/141-class-unload/src-ex/IntHolder.java
+++ b/test/141-class-unload/src-ex/IntHolder.java
@@ -36,4 +36,8 @@
}
public static native void waitForCompilation();
+
+ public static Throwable generateStackTrace() {
+ return new Exception("test");
+ }
}
diff --git a/test/141-class-unload/src/Main.java b/test/141-class-unload/src/Main.java
index 105a2b9..3cc43ac 100644
--- a/test/141-class-unload/src/Main.java
+++ b/test/141-class-unload/src/Main.java
@@ -39,6 +39,8 @@
testNoUnloadInstance(constructor);
// Test JNI_OnLoad and JNI_OnUnload.
testLoadAndUnloadLibrary(constructor);
+ // Test that stack traces keep the classes live.
+ testStackTrace(constructor);
// Stress test to make sure we dont leak memory.
stressTest(constructor);
} catch (Exception e) {
@@ -75,6 +77,16 @@
System.out.println(loader.get());
}
+ private static void testStackTrace(Constructor constructor) throws Exception {
+ WeakReference<Class> klass = setUpUnloadClass(constructor);
+ Method stackTraceMethod = klass.get().getDeclaredMethod("generateStackTrace");
+ Throwable throwable = (Throwable) stackTraceMethod.invoke(klass.get());
+ stackTraceMethod = null;
+ Runtime.getRuntime().gc();
+ boolean isNull = klass.get() == null;
+ System.out.println("class null " + isNull + " " + throwable.getMessage());
+ }
+
private static void testLoadAndUnloadLibrary(Constructor constructor) throws Exception {
WeakReference<ClassLoader> loader = setUpLoadLibrary(constructor);
// No strong refernces to class loader, should get unloaded.
