diff --git a/src/core/thread.d b/src/core/thread.d index df483d44e3..d596862b72 100644 --- a/src/core/thread.d +++ b/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,9 +2377,27 @@ 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 ) @@ -2391,7 +2405,7 @@ private void suspend( Thread t ) nothrow 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(); } }