suspend threads in parallel

src/core/thread.d

@@ -385,15 +385,10 @@ else version( Posix )
                 // NOTE: Since registers are being pushed and popped from the
                 //       stack, any other stack data used by this function should
                 //       be gone before the stack cleanup code is called below.
-                Thread  obj = Thread.getThis();
-
-                // NOTE: The thread reference returned by getThis is set within
-                //       the thread startup code, so it is possible that this
-                //       handler may be called before the reference is set.  In
-                //       this case it is safe to simply suspend and not worry
-                //       about the stack pointers as the thread will not have
-                //       any references to GC-managed data.
-                if( obj && !obj.m_lock )
+                Thread obj = Thread.getThis();
+                assert(obj !is null);
+
+                if( !obj.m_lock )
                 {
                     obj.m_curr.tstack = getStackTop();
                 }
@@ -407,13 +402,13 @@ else version( Posix )
                 status = sigdelset( &sigres, resumeSignalNumber );
                 assert( status == 0 );
 
-                version (FreeBSD) Thread.sm_suspendagain = false;
+                version (FreeBSD) obj.m_suspendagain = false;
                 status = sem_post( &suspendCount );
                 assert( status == 0 );
 
                 sigsuspend( &sigres );
 
-                if( obj && !obj.m_lock )
+                if( !obj.m_lock )
                 {
                     obj.m_curr.tstack = obj.m_curr.bstack;
                 }
@@ -422,9 +417,10 @@ else version( Posix )
             // avoid deadlocks on FreeBSD, see Issue 13416
             version (FreeBSD)
             {
-                if (THR_IN_CRITICAL(pthread_self()))
+                auto obj = Thread.getThis();
+                if (THR_IN_CRITICAL(obj.m_addr))
                 {
-                    Thread.sm_suspendagain = true;
+                    obj.m_suspendagain = true;
                     if (sem_post(&suspendCount)) assert(0);
                     return;
                 }
@@ -1400,7 +1396,7 @@ private:
     version (FreeBSD)
     {
         // set when suspend failed and should be retried, see Issue 13416
-        static shared bool sm_suspendagain;
+        shared bool m_suspendagain;
     }
 
 
@@ -2381,17 +2377,35 @@ private __gshared uint suspendDepth = 0;
  *
  * Throws:
  *  ThreadError if the suspend operation fails for a running thread.
+ * Returns:
+ *  Whether the thread is now suspended (true) or terminated (false).
  */
-private void suspend( Thread t ) nothrow
+private bool suspend( Thread t ) nothrow
 {
+    Duration waittime = dur!"usecs"(10);
+ Lagain:
+    if (!t.isRunning)
+    {
+        Thread.remove(t);
+        return false;
+    }
+    else if (t.m_isInCriticalRegion)
+    {
+        Thread.criticalRegionLock.unlock_nothrow();
+        Thread.sleep(waittime);
+        if (waittime < dur!"msecs"(10)) waittime *= 2;
+        Thread.criticalRegionLock.lock_nothrow();
+        goto Lagain;
+    }
+
     version( Windows )
     {
         if( t.m_addr != GetCurrentThreadId() && SuspendThread( t.m_hndl ) == 0xFFFFFFFF )
         {
             if( !t.isRunning )
             {
                 Thread.remove( t );
-                return;
+                return false;
             }
             onThreadError( "Unable to suspend thread" );
         }
@@ -2450,7 +2464,7 @@ private void suspend( Thread t ) nothrow
             if( !t.isRunning )
             {
                 Thread.remove( t );
-                return;
+                return false;
             }
             onThreadError( "Unable to suspend thread" );
         }
@@ -2511,33 +2525,22 @@ private void suspend( Thread t ) nothrow
     {
         if( t.m_addr != pthread_self() )
         {
-        Lagain:
             if( pthread_kill( t.m_addr, suspendSignalNumber ) != 0 )
             {
                 if( !t.isRunning )
                 {
                     Thread.remove( t );
-                    return;
+                    return false;
                 }
                 onThreadError( "Unable to suspend thread" );
             }
-            while (sem_wait(&suspendCount) != 0)
-            {
-                if (errno != EINTR)
-                    onThreadError( "Unable to wait for semaphore" );
-                errno = 0;
-            }
-            version (FreeBSD)
-            {
-                // avoid deadlocks, see Issue 13416
-                if (Thread.sm_suspendagain) goto Lagain;
-            }
         }
         else if( !t.m_lock )
         {
             t.m_curr.tstack = getStackTop();
         }
     }
+    return true;
 }
 
 /**
@@ -2577,40 +2580,51 @@ extern (C) void thread_suspendAll() nothrow
         if( ++suspendDepth > 1 )
             return;
 
-        // NOTE: I'd really prefer not to check isRunning within this loop but
-        //       not doing so could be problematic if threads are terminated
-        //       abnormally and a new thread is created with the same thread
-        //       address before the next GC run.  This situation might cause
-        //       the same thread to be suspended twice, which would likely
-        //       cause the second suspend to fail, the garbage collection to
-        //       abort, and Bad Things to occur.
-
         Thread.criticalRegionLock.lock_nothrow();
+        scope (exit) Thread.criticalRegionLock.unlock_nothrow();
+        size_t cnt;
         auto t = Thread.sm_tbeg;
         while (t)
         {
             auto tn = t.next;
-            Duration waittime = dur!"usecs"(10);
+            if (suspend(t))
+                ++cnt;
+            t = tn;
+        }
+
+        version (OSX)
+        {}
+        else version (Posix)
+        {
+            // subtract own thread
+            assert(cnt >= 1);
+            --cnt;
         Lagain:
-            if (!t.isRunning)
+            // wait for semaphore notifications
+            for (; cnt; --cnt)
             {
-                Thread.remove(t);
-            }
-            else if (t.m_isInCriticalRegion)
-            {
-                Thread.criticalRegionLock.unlock_nothrow();
-                Thread.sleep(waittime);
-                if (waittime < dur!"msecs"(10)) waittime *= 2;
-                Thread.criticalRegionLock.lock_nothrow();
-                goto Lagain;
+                while (sem_wait(&suspendCount) != 0)
+                {
+                    if (errno != EINTR)
+                        onThreadError("Unable to wait for semaphore");
+                    errno = 0;
+                }
             }
-            else
+            version (FreeBSD)
             {
-                suspend(t);
-            }
-            t = tn;
+                // avoid deadlocks, see Issue 13416
+                t = Thread.sm_tbeg;
+                while (t)
+                {
+                    auto tn = t.next;
+                    if (t.m_suspendagain && suspend(t))
+                        ++cnt;
+                    t = tn;
+                }
+                if (cnt)
+                    goto Lagain;
+             }
         }
-        Thread.criticalRegionLock.unlock_nothrow();
     }
 }