Release the thread_list lock while waiting for daemons.

Daemons might be in a state where they are actually waiting for
it (for example Thread.isInterrupted). If the shutdown thread
doesn't release the lock, such daemon cannot make progress.

bug:27353286
Change-Id: Ib7f4c7d1b56d73a829d01d5bfc0ab663fbb80b46

diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index d90bd8d..97bcb7d 100644 (file)
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -1146,9 +1146,10 @@ void ThreadList::WaitForOtherNonDaemonThreadsToExit() {
 void ThreadList::SuspendAllDaemonThreadsForShutdown() {
   ScopedTrace trace(__PRETTY_FUNCTION__);
   Thread* self = Thread::Current();
-  MutexLock mu(self, *Locks::thread_list_lock_);
   size_t daemons_left = 0;
-  {  // Tell all the daemons it's time to suspend.
+  {
+    // Tell all the daemons it's time to suspend.
+    MutexLock mu(self, *Locks::thread_list_lock_);
     MutexLock mu2(self, *Locks::thread_suspend_count_lock_);
     for (const auto& thread : list_) {
       // This is only run after all non-daemon threads have exited, so the remainder should all be
@@ -1177,13 +1178,16 @@ void ThreadList::SuspendAllDaemonThreadsForShutdown() {
   static constexpr size_t kSleepMicroseconds = 1000;
   for (size_t i = 0; i < kTimeoutMicroseconds / kSleepMicroseconds; ++i) {
     bool all_suspended = true;
-    for (const auto& thread : list_) {
-      if (thread != self && thread->GetState() == kRunnable) {
-        if (!have_complained) {
-          LOG(WARNING) << "daemon thread not yet suspended: " << *thread;
-          have_complained = true;
+    {
+      MutexLock mu(self, *Locks::thread_list_lock_);
+      for (const auto& thread : list_) {
+        if (thread != self && thread->GetState() == kRunnable) {
+          if (!have_complained) {
+            LOG(WARNING) << "daemon thread not yet suspended: " << *thread;
+            have_complained = true;
+          }
+          all_suspended = false;
         }
-        all_suspended = false;
       }
     }
     if (all_suspended) {

diff --git a/test/607-daemon-stress/expected.txt b/test/607-daemon-stress/expected.txt
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/test/607-daemon-stress/info.txt b/test/607-daemon-stress/info.txt
new file mode 100644 (file)
index 0000000..1047b76
--- /dev/null
+++ b/test/607-daemon-stress/info.txt
@@ -0,0 +1,3 @@
+Stress test for daemon threads stuck in a method that requires the thread list lock.
+(for example Thread.isInterrupted). The shutdown thread used to block those daemons
+from making progress.

diff --git a/test/607-daemon-stress/src/Main.java b/test/607-daemon-stress/src/Main.java
new file mode 100644 (file)
index 0000000..56ef410
--- /dev/null
+++ b/test/607-daemon-stress/src/Main.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+public class Main extends Thread {
+  public static void main(String[] args) throws Exception {
+    for (int i = 0; i < 5; i++) {
+      Main m = new Main();
+      m.setDaemon(true);
+      m.start();
+    }
+    // Sleep a while to give some time for the threads to start.
+    Thread.sleep(1000);
+  }
+
+  public void run() {
+    while (!isInterrupted());
+  }
+}