From 07dcbf8f2cfc6acab973c2109147b45f8107d67e Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:25:32 +0300 Subject: [PATCH 01/13] First commit --- src/core/thread/osthread.d | 24 ++++- src/core/thread/threadbase.d | 30 ++++++ test/thread/Makefile | 7 +- test/thread/src/attach_detach.d | 160 ++++++++++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 6 deletions(-) create mode 100644 test/thread/src/attach_detach.d diff --git a/src/core/thread/osthread.d b/src/core/thread/osthread.d index 93bd56a522..ec9768eaee 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) { @@ -657,6 +661,7 @@ class Thread : ThreadBase // by definition result.PRIORITY_DEFAULT = 0; } + m_isOwned = true; } else version (Posix) { @@ -1248,6 +1253,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 +1273,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..9f60dbd0ce 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 { + Thread.slock.lock_nothrow(); + scope(exit) Thread.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) { + Thread.slock.lock_nothrow(); + scope(exit) Thread.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 { + Thread.slock.lock_nothrow(); + scope(exit) Thread.slock.unlock_nothrow(); + ThreadBase.remove(t); } @@ -940,6 +958,18 @@ extern (C) void thread_joinAll() ThreadBase.slock.unlock_nothrow(); } +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(); +} /** * Performs intermediate shutdown of the thread module. 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() +{ +} + +} From a57d9d6748d75733807609c66d78cb86ea18bf84 Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:28:29 +0300 Subject: [PATCH 02/13] Update osthread.d --- src/core/thread/osthread.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/thread/osthread.d b/src/core/thread/osthread.d index ec9768eaee..a5984b02f6 100644 --- a/src/core/thread/osthread.d +++ b/src/core/thread/osthread.d @@ -216,7 +216,7 @@ class Thread : ThreadBase version (Posix) { private shared bool m_isRunning; - private bool m_isOwned; + private bool m_isOwned; } version (Darwin) From 77fe6a26ce55f147b4f83a2b5fdbf2bd6078f2b9 Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:29:11 +0300 Subject: [PATCH 03/13] Update osthread.d --- src/core/thread/osthread.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/thread/osthread.d b/src/core/thread/osthread.d index a5984b02f6..b29754298b 100644 --- a/src/core/thread/osthread.d +++ b/src/core/thread/osthread.d @@ -303,7 +303,7 @@ class Thread : ThreadBase if (m_isOwned) { if (m_addr != m_addr.init) - pthread_detach( m_addr ); + pthread_detach( m_addr ); m_addr = m_addr.init; } } From 8c9ae4c36582426e9cacbed953e834198012a373 Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:29:58 +0300 Subject: [PATCH 04/13] Update osthread.d --- src/core/thread/osthread.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/thread/osthread.d b/src/core/thread/osthread.d index b29754298b..5d0bab736a 100644 --- a/src/core/thread/osthread.d +++ b/src/core/thread/osthread.d @@ -1253,7 +1253,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_isOwned = false; } thisThread.m_isDaemon = true; thisThread.tlsGCdataInit(); From da76c3eb3b8e217e2ddb22c2de5680b8078c4ecc Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:30:46 +0300 Subject: [PATCH 05/13] Update threadbase.d --- src/core/thread/threadbase.d | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/thread/threadbase.d b/src/core/thread/threadbase.d index 9f60dbd0ce..af220f8af7 100644 --- a/src/core/thread/threadbase.d +++ b/src/core/thread/threadbase.d @@ -836,10 +836,10 @@ extern (C) void thread_detachThis() nothrow @nogc scope(exit) Thread.slock.unlock_nothrow(); if (auto t = ThreadBase.getThis()) - { + { ThreadBase.remove(t); - t.setThis(null); - } + t.setThis(null); + } } From 0f509aeb24c280410e8962147c937c6669ce6083 Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:36:56 +0300 Subject: [PATCH 06/13] Update threadbase.d --- src/core/thread/threadbase.d | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core/thread/threadbase.d b/src/core/thread/threadbase.d index af220f8af7..2003a41632 100644 --- a/src/core/thread/threadbase.d +++ b/src/core/thread/threadbase.d @@ -832,8 +832,8 @@ package ThreadT thread_attachThis_tpl(ThreadT)() */ extern (C) void thread_detachThis() nothrow @nogc { - Thread.slock.lock_nothrow(); - scope(exit) Thread.slock.unlock_nothrow(); + ThreadBase.slock.lock_nothrow(); + scope(exit) ThreadBase.slock.unlock_nothrow(); if (auto t = ThreadBase.getThis()) { @@ -856,8 +856,8 @@ extern (C) void thread_detachThis() nothrow @nogc */ extern (C) void thread_detachByAddr(ThreadID addr) { - Thread.slock.lock_nothrow(); - scope(exit) Thread.slock.unlock_nothrow(); + ThreadBase.slock.lock_nothrow(); + scope(exit) ThreadBase.slock.unlock_nothrow(); if (auto t = thread_findByAddr(addr)) ThreadBase.remove(t); @@ -867,8 +867,8 @@ extern (C) void thread_detachByAddr(ThreadID addr) /// ditto extern (C) void thread_detachInstance(ThreadBase t) nothrow @nogc { - Thread.slock.lock_nothrow(); - scope(exit) Thread.slock.unlock_nothrow(); + ThreadBase.slock.lock_nothrow(); + scope(exit) ThreadBase.slock.unlock_nothrow(); ThreadBase.remove(t); } From d4693a11fc05effda06f55382271339590829ae8 Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:40:04 +0300 Subject: [PATCH 07/13] Update osthread.d --- src/core/thread/osthread.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/thread/osthread.d b/src/core/thread/osthread.d index 5d0bab736a..1a70c20957 100644 --- a/src/core/thread/osthread.d +++ b/src/core/thread/osthread.d @@ -496,6 +496,7 @@ class Thread : ThreadBase } if ( pthread_attr_destroy( &attr ) != 0 ) onThreadError( "Error destroying thread attributes" ); + m_isOwned = true; } version (Darwin) { @@ -661,7 +662,6 @@ class Thread : ThreadBase // by definition result.PRIORITY_DEFAULT = 0; } - m_isOwned = true; } else version (Posix) { From 6a628b4ddf9a21d72d6dd5f4dfb8d7dcf5592e1a Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:47:58 +0300 Subject: [PATCH 08/13] Update threadbase.d --- src/core/thread/threadbase.d | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/core/thread/threadbase.d b/src/core/thread/threadbase.d index 2003a41632..18b6ba1250 100644 --- a/src/core/thread/threadbase.d +++ b/src/core/thread/threadbase.d @@ -958,19 +958,6 @@ extern (C) void thread_joinAll() ThreadBase.slock.unlock_nothrow(); } -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(); -} - /** * Performs intermediate shutdown of the thread module. */ From 011a53d300a22ed5d322b49f5f93b68b60025eba Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:49:53 +0300 Subject: [PATCH 09/13] Update threadbase.d --- src/core/thread/threadbase.d | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/core/thread/threadbase.d b/src/core/thread/threadbase.d index 18b6ba1250..098b103ecb 100644 --- a/src/core/thread/threadbase.d +++ b/src/core/thread/threadbase.d @@ -916,6 +916,18 @@ extern (C) void thread_setThis(ThreadBase t) nothrow @nogc ThreadBase.setThis(t); } +unittest +{ + auto t = new ThreadBase( + { + auto old = ThreadBase.getThis(); + assert(old !is null); + thread_setThis(null); + assert(Thread.getThis() is null); + thread_setThis(old); + }).start; + t.join(); +} /** * Joins all non-daemon threads that are currently running. This is done by From c500747912c57e29854885ff9251735621fd23a0 Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:50:05 +0300 Subject: [PATCH 10/13] Update threadbase.d --- src/core/thread/threadbase.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/thread/threadbase.d b/src/core/thread/threadbase.d index 098b103ecb..8f51320153 100644 --- a/src/core/thread/threadbase.d +++ b/src/core/thread/threadbase.d @@ -923,7 +923,7 @@ unittest auto old = ThreadBase.getThis(); assert(old !is null); thread_setThis(null); - assert(Thread.getThis() is null); + assert(ThreadBase.getThis() is null); thread_setThis(old); }).start; t.join(); From 7c344cbe9be67476a6641114ac8da2e3dfa152ef Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 17:50:29 +0300 Subject: [PATCH 11/13] Update threadbase.d --- src/core/thread/threadbase.d | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/thread/threadbase.d b/src/core/thread/threadbase.d index 8f51320153..a9cf8be740 100644 --- a/src/core/thread/threadbase.d +++ b/src/core/thread/threadbase.d @@ -970,6 +970,7 @@ extern (C) void thread_joinAll() ThreadBase.slock.unlock_nothrow(); } + /** * Performs intermediate shutdown of the thread module. */ From a4b6c63237200d8e69c170c5bf44443c2a0aa50f Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 18:20:32 +0300 Subject: [PATCH 12/13] Update threadbase.d --- src/core/thread/threadbase.d | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/core/thread/threadbase.d b/src/core/thread/threadbase.d index a9cf8be740..74118d4ef4 100644 --- a/src/core/thread/threadbase.d +++ b/src/core/thread/threadbase.d @@ -916,18 +916,6 @@ extern (C) void thread_setThis(ThreadBase t) nothrow @nogc ThreadBase.setThis(t); } -unittest -{ - auto t = new ThreadBase( - { - auto old = ThreadBase.getThis(); - assert(old !is null); - thread_setThis(null); - assert(ThreadBase.getThis() is null); - thread_setThis(old); - }).start; - t.join(); -} /** * Joins all non-daemon threads that are currently running. This is done by From 1847abfd374530c6cdc2d2e7b8373627fb77aa89 Mon Sep 17 00:00:00 2001 From: Temtaime Date: Tue, 5 Oct 2021 18:22:18 +0300 Subject: [PATCH 13/13] Update osthread.d --- src/core/thread/osthread.d | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/core/thread/osthread.d b/src/core/thread/osthread.d index 1a70c20957..989b001f30 100644 --- a/src/core/thread/osthread.d +++ b/src/core/thread/osthread.d @@ -1040,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.";