diff --git a/src/core/thread/osthread.d b/src/core/thread/osthread.d index 93bd56a522..989b001f30 100644 --- a/src/core/thread/osthread.d +++ b/src/core/thread/osthread.d @@ -216,6 +216,7 @@ class Thread : ThreadBase version (Posix) { private shared bool m_isRunning; + private bool m_isOwned; } version (Darwin) @@ -299,9 +300,12 @@ class Thread : ThreadBase } else version (Posix) { - if (m_addr != m_addr.init) - pthread_detach( m_addr ); - m_addr = m_addr.init; + if (m_isOwned) + { + if (m_addr != m_addr.init) + pthread_detach( m_addr ); + m_addr = m_addr.init; + } } version (Darwin) { @@ -492,6 +496,7 @@ class Thread : ThreadBase } if ( pthread_attr_destroy( &attr ) != 0 ) onThreadError( "Error destroying thread attributes" ); + m_isOwned = true; } version (Darwin) { @@ -1035,6 +1040,20 @@ unittest } +unittest +{ + auto t = new Thread( + { + auto old = Thread.getThis(); + assert(old !is null); + thread_setThis(null); + assert(Thread.getThis() is null); + thread_setThis(old); + }).start; + t.join(); +} + + unittest { enum MSG = "Test message."; @@ -1248,6 +1267,7 @@ private extern (D) ThreadBase attachThread(ThreadBase _thisThread) @nogc nothrow thisContext.tstack = thisContext.bstack; atomicStore!(MemoryOrder.raw)(thisThread.toThread.m_isRunning, true); + thisThread.m_isOwned = false; } thisThread.m_isDaemon = true; thisThread.tlsGCdataInit(); @@ -1267,8 +1287,16 @@ private extern (D) ThreadBase attachThread(ThreadBase _thisThread) @nogc nothrow } /** - * Registers the calling thread for use with the D Runtime. If this routine - * is called for a thread which is already registered, no action is performed. + * Registers the calling thread for use with the D Runtime. If this routine is + * called for a thread which is already registered, no action is performed. On + * Posix systems, the D Runtime does not take ownership of the thread; + * specifically, it does not call `pthread_detach` during cleanup. + * + * Threads registered by this routine should normally be deregistered by + * `thread_detachThis`. Although `thread_detachByAddr` and + * `thread_detachInstance` can be used as well, such threads cannot be registered + * again by `thread_attachThis` unless `thread_setThis` is called with the + * `null` reference first. * * NOTE: This routine does not run thread-local static constructors when called. * If full functionality as a D thread is desired, the following function diff --git a/src/core/thread/threadbase.d b/src/core/thread/threadbase.d index eb2e71ee05..74118d4ef4 100644 --- a/src/core/thread/threadbase.d +++ b/src/core/thread/threadbase.d @@ -798,6 +798,12 @@ extern (C) bool thread_isMainThread() nothrow @nogc * Registers the calling thread for use with the D Runtime. If this routine * is called for a thread which is already registered, no action is performed. * + * Threads registered by this routine should normally be deregistered by $(D + * thread_detachThis). Although $(D thread_detachByAddr) and $(D + * thread_detachInstance) can be used as well, such threads cannot be registered + * again by $(D thread_attachThis) unless $(D thread_setThis) is called with the + * $(D null) value first. + * * NOTE: This routine does not run thread-local static constructors when called. * If full functionality as a D thread is desired, the following function * must be called after thread_attachThis: @@ -826,8 +832,14 @@ package ThreadT thread_attachThis_tpl(ThreadT)() */ extern (C) void thread_detachThis() nothrow @nogc { + ThreadBase.slock.lock_nothrow(); + scope(exit) ThreadBase.slock.unlock_nothrow(); + if (auto t = ThreadBase.getThis()) + { ThreadBase.remove(t); + t.setThis(null); + } } @@ -844,6 +856,9 @@ extern (C) void thread_detachThis() nothrow @nogc */ extern (C) void thread_detachByAddr(ThreadID addr) { + ThreadBase.slock.lock_nothrow(); + scope(exit) ThreadBase.slock.unlock_nothrow(); + if (auto t = thread_findByAddr(addr)) ThreadBase.remove(t); } @@ -852,6 +867,9 @@ extern (C) void thread_detachByAddr(ThreadID addr) /// ditto extern (C) void thread_detachInstance(ThreadBase t) nothrow @nogc { + ThreadBase.slock.lock_nothrow(); + scope(exit) ThreadBase.slock.unlock_nothrow(); + ThreadBase.remove(t); } diff --git a/test/thread/Makefile b/test/thread/Makefile index 6dbe948eb1..ef1c9c6e6b 100644 --- a/test/thread/Makefile +++ b/test/thread/Makefile @@ -1,6 +1,6 @@ include ../common.mak -TESTS:=fiber_guard_page external_threads tlsgc_sections test_import tlsstack join_detach +TESTS:=fiber_guard_page external_threads tlsgc_sections test_import tlsstack join_detach attach_detach .PHONY: all clean all: $(addprefix $(ROOT)/,$(addsuffix .done,$(TESTS))) @@ -34,6 +34,11 @@ $(ROOT)/tlsstack.done: $(ROOT)/%.done : $(ROOT)/% $(ROOT)/join_detach.done: $(ROOT)/%.done : $(ROOT)/% @echo Testing $* is currently disabled! +$(ROOT)/attach_detach.done: $(ROOT)/%.done : $(ROOT)/% + @echo Testing $* + $(QUIET)$(TIMELIMIT)$(ROOT)/$* + @touch $@ + $(ROOT)/%: $(SRC)/%.d $(QUIET)$(DMD) $(DFLAGS) -of$@ $< diff --git a/test/thread/src/attach_detach.d b/test/thread/src/attach_detach.d new file mode 100644 index 0000000000..55345c0206 --- /dev/null +++ b/test/thread/src/attach_detach.d @@ -0,0 +1,160 @@ +version (Posix) +{ + +import core.thread; +import core.sys.posix.pthread; +import core.stdc.stdlib; +import core.stdc.time; + +// This program creates threads that are started outside of D runtime and +// stresses attaching and detaching those threads to the D runtime. + +struct MyThread +{ + pthread_t t; + bool stopRequested; +} + +enum totalThreads = 4; + +enum runTimeSeconds = 5; // Must be less than timelimit's +MyThread[totalThreads] threads; + +auto exerciseGC() { + int[] arr; + foreach (i; 0 .. 1000) + arr ~= i; + return arr; +} + +// This represents an API function of a non-D framework. Since we don't have any +// control on the lifetime of this thread, we have to attach upon entry and +// detach upon exit. +void api_foo() +{ + auto t = thread_attachThis(); + scope(exit) + { + // Pick a detachment method + final switch (rand() % 3) + { + case 0: + thread_detachThis(); + break; + case 1: + thread_detachByAddr(t.id); + // thread_setThis must be called by the detached thread; it happens + // to be the case in this test. + thread_setThis(null); + break; + case 2: + thread_detachInstance(t); + // thread_setThis must be called by the detached thread; it happens + // to be the case in this test. + thread_setThis(null); + break; + } + } + + assert_thread_is_attached(t.id); + cast(void)exerciseGC(); +} + +// Make calls to an api function and exit when requested +extern(C) void * thread_func(void * arg) +{ + MyThread *t = cast(MyThread*)arg; + + while (!t.stopRequested) + api_foo(); + + return arg; +} + +void start_thread(ref MyThread t) +{ + pthread_attr_t attr; + int err = pthread_attr_init(&attr); + assert(!err); + + t.stopRequested = false; + err = pthread_create(&t.t, &attr, &thread_func, cast(void*)&t); + assert(!err); + + err = pthread_attr_destroy(&attr); + assert(!err); +} + +void start_threads() +{ + foreach (ref t; threads) + start_thread(t); +} + +void stop_thread(ref MyThread t) +{ + t.stopRequested = true; + const err = pthread_join(t.t, null); + assert(!err); + + assert_thread_is_gone(t.t); +} + +void stop_threads() +{ + foreach (ref t; threads) + stop_thread(t); +} + +void assert_thread_is_attached(pthread_t tid) +{ + size_t found = 0; + foreach (t; Thread.getAll()) + if (tid == t.id) + { + ++found; + } + assert(found == 1); +} + +void assert_thread_is_gone(pthread_t tid) +{ + foreach (t; Thread.getAll()) + assert(tid != t.id); +} + +// Occasionally stop threads and start new ones +void watch_threads() +{ + const start = time(null); + + while ((time(null) - start) < runTimeSeconds) + { + foreach (ref t; threads) + { + const shouldStop = ((rand() % 100) == 0); + if (shouldStop) + { + stop_thread(t); + start_thread(t); + } + } + } +} + +void main() +{ + start_threads(); + watch_threads(); + stop_threads(); +} + +} // version (Posix) +else +{ + +void main() +{ +} + +}