Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Commit

Permalink
fix Issue 15268 - deadlock for Thread.getAll/Thread.opApply
Browse files Browse the repository at this point in the history
- fix a deadlock caused by lock order inversion
- must not use the GC while holding slock
  • Loading branch information
MartinNowak committed Oct 31, 2015
1 parent be0c48e commit d56a259
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/core/exception.d
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ private T staticError(T, Args...)(auto ref Args args)

// Suppress traceinfo generation when the GC cannot be used. Workaround for
// Bugzilla 14993. We should make stack traces @nogc instead.
private class SuppressTraceInfo : Throwable.TraceInfo
package class SuppressTraceInfo : Throwable.TraceInfo
{
override int opApply(scope int delegate(ref const(char[]))) const { return 0; }
override int opApply(scope int delegate(ref size_t, ref const(char[]))) const { return 0; }
Expand Down
80 changes: 61 additions & 19 deletions src/core/thread.d
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,8 @@ class Thread

/**
* Provides a list of all threads currently being tracked by the system.
* Note that threads in the returned array might no longer run (see
* $(D Thread.)$(LREF isRunning)).
*
* Returns:
* An array containing references to all threads currently being
Expand All @@ -1170,46 +1172,81 @@ class Thread
*/
static Thread[] getAll()
{
synchronized( slock )
static void resize(ref Thread[] buf, size_t nlen)
{
size_t pos = 0;
Thread[] buf = new Thread[sm_tlen];

foreach( Thread t; Thread )
{
buf[pos++] = t;
}
return buf;
buf.length = nlen;
}
return getAllImpl!resize();
}


/**
* Operates on all threads currently being tracked by the system. The
* result of deleting any Thread object is undefined.
* Note that threads passed to the callback might no longer run (see
* $(D Thread.)$(LREF isRunning)).
*
* Params:
* dg = The supplied code as a delegate.
*
* Returns:
* Zero if all elemented are visited, nonzero if not.
*/
static int opApply( scope int delegate( ref Thread ) dg )
static int opApply(scope int delegate(ref Thread) dg)
{
synchronized( slock )
import core.stdc.stdlib : free, realloc;

static void resize(ref Thread[] buf, size_t nlen)
{
auto t = sm_tbeg;
while (t)
{
auto tn = t.next;
if (auto ret = dg(t))
return ret;
t = tn;
}
buf = (cast(Thread*)realloc(buf.ptr, nlen * Thread.sizeof))[0 .. nlen];
}
auto buf = getAllImpl!resize;
scope(exit) if (buf.ptr) free(buf.ptr);

foreach (t; buf)
{
if (auto res = dg(t))
return res;
}
return 0;
}

unittest
{
auto t1 = new Thread({
foreach (_; 0 .. 20)
Thread.getAll;
}).start;
auto t2 = new Thread({
foreach (_; 0 .. 20)
GC.collect;
}).start;
t1.join();
t2.join();
}

private static Thread[] getAllImpl(alias resize)()
{
import core.atomic;

Thread[] buf;
while (true)
{
immutable len = atomicLoad!(MemoryOrder.raw)(*cast(shared)&sm_tlen);
resize(buf, len);
assert(buf.length == len);
synchronized (slock)
{
if (len == sm_tlen)
{
size_t pos;
for (Thread t = sm_tbeg; t; t = t.next)
buf[pos++] = t;
return buf;
}
}
}
}

///////////////////////////////////////////////////////////////////////////
// Static Initalizer
Expand Down Expand Up @@ -1605,6 +1642,9 @@ private:
//
// All use of the global lists should synchronize on this lock.
//
// Careful as the GC acquires this lock after the GC lock to suspend all
// threads any GC usage with slock held can result in a deadlock through
// lock order inversion.
@property static Mutex slock() nothrow
{
return cast(Mutex)_locks[0].ptr;
Expand Down Expand Up @@ -2942,6 +2982,8 @@ private void onThreadError(string msg = null, Throwable next = null) nothrow
__gshared ThreadError error = new ThreadError(null);
error.msg = msg;
error.next = next;
import core.exception : SuppressTraceInfo;
error.info = SuppressTraceInfo.instance;
throw error;
}

Expand Down

0 comments on commit d56a259

Please sign in to comment.