Avoid suspending heap task thread for getting stack traces

Instead of suspending the heap task thread, GetThreadStack (called by
VMStack_fillStackTraceElements and VMStack_getThreadStackTrace) will
return an empty thread stack. This fixes possible deadlocks caused by
suspending the GC thread and doing allocations for the stack trace.

Bug: 28261069

Test: test-art-host

Change-Id: I45a0b8ac94a99d6bbcfcdc2b41afadf941ec0138

diff --git a/runtime/native/dalvik_system_VMStack.cc b/runtime/native/dalvik_system_VMStack.cc
index 36825cb..268d71a 100644 (file)
--- a/runtime/native/dalvik_system_VMStack.cc
+++ b/runtime/native/dalvik_system_VMStack.cc
@@ -17,6 +17,7 @@
 #include "dalvik_system_VMStack.h"
 
 #include "art_method-inl.h"
+#include "gc/task_processor.h"
 #include "jni_internal.h"
 #include "nth_caller_visitor.h"
 #include "mirror/class-inl.h"
@@ -31,9 +32,18 @@ namespace art {
 static jobject GetThreadStack(const ScopedFastNativeObjectAccess& soa, jobject peer)
     REQUIRES_SHARED(Locks::mutator_lock_) {
   jobject trace = nullptr;
-  if (soa.Decode<mirror::Object>(peer) == soa.Self()->GetPeer()) {
+  ObjPtr<mirror::Object> decoded_peer = soa.Decode<mirror::Object>(peer);
+  if (decoded_peer == soa.Self()->GetPeer()) {
     trace = soa.Self()->CreateInternalStackTrace<false>(soa);
   } else {
+    // Never allow suspending the heap task thread since it may deadlock if allocations are
+    // required for the stack trace.
+    Thread* heap_task_thread =
+        Runtime::Current()->GetHeap()->GetTaskProcessor()->GetRunningThread();
+    // heap_task_thread could be null if the daemons aren't yet started.
+    if (heap_task_thread != nullptr && decoded_peer == heap_task_thread->GetPeer()) {
+      return nullptr;
+    }
     // Suspend thread to build stack trace.
     ScopedThreadSuspension sts(soa.Self(), kNative);
     ThreadList* thread_list = Runtime::Current()->GetThreadList();

diff --git a/test/129-ThreadGetId/expected.txt b/test/129-ThreadGetId/expected.txt
index 134d8d0..aadf90d 100644 (file)
--- a/test/129-ThreadGetId/expected.txt
+++ b/test/129-ThreadGetId/expected.txt
@@ -1 +1,2 @@
+HeapTaskDaemon depth 0
 Finishing

diff --git a/test/129-ThreadGetId/src/Main.java b/test/129-ThreadGetId/src/Main.java
index 9934bba..5aefd17 100644 (file)
--- a/test/129-ThreadGetId/src/Main.java
+++ b/test/129-ThreadGetId/src/Main.java
@@ -22,6 +22,7 @@ public class Main implements Runnable {
 
     public static void main(String[] args) throws Exception {
         final Thread[] threads = new Thread[numberOfThreads];
+        test_getStackTraces();
         for (int t = 0; t < threads.length; t++) {
             threads[t] = new Thread(new Main());
             threads[t].start();
@@ -32,6 +33,19 @@ public class Main implements Runnable {
         System.out.println("Finishing");
     }
 
+    static void test_getStackTraces() {
+        // Check all the current threads for positive IDs.
+        Map<Thread, StackTraceElement[]> map = Thread.getAllStackTraces();
+        for (Map.Entry<Thread, StackTraceElement[]> pair : map.entrySet()) {
+            Thread thread = pair.getKey();
+            // Expect empty stack trace since we do not support suspending the GC thread for
+            // obtaining stack traces. See b/28261069.
+            if (thread.getName().equals("HeapTaskDaemon")) {
+                System.out.println(thread.getName() + " depth " + pair.getValue().length); 
+            }
+        }
+    }
+
     public void test_getId() {
         if (Thread.currentThread().getId() <= 0) {
             System.out.println("current thread's ID is not positive");