commit e99f53203904c23a26e53ca1bf6a4e45814146fe
author Mathieu Chartier <mathieuc@google.com> Fri Jun 10 17:04:20 2016 -0700
committer Mathieu Chartier <mathieuc@google.com> Fri Jun 10 17:52:36 2016 -0700
tree 88142212532717ef4e2435420493eeab02d1eb11
parent 4811e726711e42781e7b9d56763588cef796e2f0

Use ScopedObjectAccess in ThreadList::Dump

Prevent deadlocks if two different threads are calling Dump at the
same time:

Thread 1: Requests thread 2 to suspend
Thread 2: Requests thread 1 to suspend
Both threads are suspended and blocked on ScopedObjectAccess in
thread dumping.

Added to suspend all stress, reduced thread count to 4 temporarily
since this is the maximum number of simultaneous checkpoints.

Bug: 28988206

Change-Id: I9e4b8391ebad0bca0e42a03819c58c99ddba6b35

runtime/thread_list.cc

diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index da21479..80b99fc 100644
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -234,7 +234,12 @@
     os << "DALVIK THREADS (" << list_.size() << "):\n";
   }
   DumpCheckpoint checkpoint(&os, dump_native_stack);
-  size_t threads_running_checkpoint = RunCheckpoint(&checkpoint);
+  size_t threads_running_checkpoint;
+  {
+    // Use SOA to prevent deadlocks if multiple threads are calling Dump() at the same time.
+    ScopedObjectAccess soa(Thread::Current());
+    threads_running_checkpoint = RunCheckpoint(&checkpoint);
+  }
   if (threads_running_checkpoint != 0) {
     checkpoint.WaitForThreadsToRunThroughCheckpoint(threads_running_checkpoint);
   }

test/149-suspend-all-stress/src/Main.java

diff --git a/test/149-suspend-all-stress/src/Main.java b/test/149-suspend-all-stress/src/Main.java
index 6a27c4b..aa94fc9 100644
--- a/test/149-suspend-all-stress/src/Main.java
+++ b/test/149-suspend-all-stress/src/Main.java
@@ -17,7 +17,7 @@
 import java.util.Map;
 
 public class Main implements Runnable {
-    static final int numberOfThreads = 8;
+    static final int numberOfThreads = 4;
 
     public static void main(String[] args) throws Exception {
         System.loadLibrary(args[0]);

test/149-suspend-all-stress/suspend_all.cc

diff --git a/test/149-suspend-all-stress/suspend_all.cc b/test/149-suspend-all-stress/suspend_all.cc
index c22ddad..de479a5 100644
--- a/test/149-suspend-all-stress/suspend_all.cc
+++ b/test/149-suspend-all-stress/suspend_all.cc
@@ -21,11 +21,37 @@
 namespace art {
 
 extern "C" JNIEXPORT void JNICALL Java_Main_suspendAndResume(JNIEnv*, jclass) {
-  usleep(100 * 1000);  // Leave some time for threads to get in here before we start suspending.
-  for (size_t i = 0; i < 500; ++i) {
-    Runtime::Current()->GetThreadList()->SuspendAll(__FUNCTION__);
-    usleep(500);
-    Runtime::Current()->GetThreadList()->ResumeAll();
+  static constexpr size_t kInitialSleepUS = 100 * 1000;  // 100ms.
+  static constexpr size_t kIterations = 500;
+  usleep(kInitialSleepUS);  // Leave some time for threads to get in here before we start suspending.
+  enum Operation {
+    kOPSuspendAll,
+    kOPDumpStack,
+    kOPSuspendAllDumpStack,
+    // Total number of operations.
+    kOPNumber,
+  };
+  for (size_t i = 0; i < kIterations; ++i) {
+    switch (static_cast<Operation>(i % kOPNumber)) {
+      case kOPSuspendAll: {
+        ScopedSuspendAll ssa(__FUNCTION__);
+        usleep(500);
+        break;
+      }
+      case kOPDumpStack: {
+        Runtime::Current()->GetThreadList()->Dump(LOG(INFO));
+        usleep(500);
+        break;
+      }
+      case kOPSuspendAllDumpStack: {
+        // Not yet supported.
+        // ScopedSuspendAll ssa(__FUNCTION__);
+        // Runtime::Current()->GetThreadList()->Dump(LOG(INFO));
+        break;
+      }
+      case kOPNumber:
+        break;
+    }
   }
 }