From f60eb358ccbc14a1a5fc1774eab505ed0132e999 Mon Sep 17 00:00:00 2001 From: Mihails Strasuns Date: Mon, 22 Oct 2018 12:57:38 +0300 Subject: [PATCH 1/2] Fix crash in destructor of detached thread Fix issue 19314 Expectation is that after thread is detached from druntime, it becomes responsibility of external environment to terminate it properly (for example, by calling `join` from C/C++ code). However, `Thread` object for that thread will still remain and will eventually be collected by GC. Destructor tries to call `pthread_detach` unconditionally but it is undefined behaviour if thread was already joined before (see `man pthread_detach`). --- src/core/thread.d | 5 +++- test/thread/src/external_threads.d | 38 ++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/core/thread.d b/src/core/thread.d index fe05fc3714..0f4c68f955 100644 --- a/src/core/thread.d +++ b/src/core/thread.d @@ -596,7 +596,10 @@ class Thread */ ~this() nothrow @nogc { - if ( m_addr == m_addr.init ) + bool no_context = m_addr == m_addr.init; + bool not_registered = !next && !prev && (sm_tbeg !is this); + + if (no_context || not_registered) { return; } diff --git a/test/thread/src/external_threads.d b/test/thread/src/external_threads.d index 087c4bfebd..9c98a3fa13 100644 --- a/test/thread/src/external_threads.d +++ b/test/thread/src/external_threads.d @@ -1,8 +1,12 @@ import core.sys.posix.pthread; import core.memory; +import core.thread; + +extern (C) void rt_moduleTlsCtor(); +extern (C) void rt_moduleTlsDtor(); extern(C) -void* entry_point(void*) +void* entry_point1(void*) { // try collecting - GC must ignore this call because this thread // is not registered in runtime @@ -10,13 +14,37 @@ void* entry_point(void*) return null; } +extern(C) +void* entry_point2(void*) +{ + // This thread gets registered in druntime, does some work and gets + // unregistered to be cleaned up manually + thread_attachThis(); + rt_moduleTlsCtor(); + + auto x = new int[10]; + + rt_moduleTlsDtor(); + thread_detachThis(); + return null; +} + void main() { // allocate some garbage auto x = new int[1000]; - pthread_t thread; - auto status = pthread_create(&thread, null, &entry_point, null); - assert(status == 0); - pthread_join(thread, null); + { + pthread_t thread; + auto status = pthread_create(&thread, null, &entry_point1, null); + assert(status == 0); + pthread_join(thread, null); + } + + { + pthread_t thread; + auto status = pthread_create(&thread, null, &entry_point2, null); + assert(status == 0); + pthread_join(thread, null); + } } From f0abd064f46f5b6e57e48ff4572b28b68afba7dc Mon Sep 17 00:00:00 2001 From: Mihails Strasuns Date: Wed, 24 Oct 2018 10:23:20 +0100 Subject: [PATCH 2/2] Unregister thread even if is the last one Previously `remove` implementation would early return if removed thread is the only element in the registered list (and thus has no prev/next references) --- src/core/thread.d | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/thread.d b/src/core/thread.d index 0f4c68f955..f402d0939a 100644 --- a/src/core/thread.d +++ b/src/core/thread.d @@ -1859,8 +1859,9 @@ private: do { // Thread was already removed earlier, might happen b/c of thread_detachInstance - if (!t.next && !t.prev) + if (!t.next && !t.prev && (sm_tbeg !is t)) return; + slock.lock_nothrow(); { // NOTE: When a thread is removed from the global thread list its