From 42b4e0a9614ac794d4549ed5b2455fd0f805e123 Mon Sep 17 00:00:00 2001
From: Mihails Strasuns <mihails.strasuns@gmail.com>
Date: Thu, 18 Oct 2018 10:57:40 +0300
Subject: [PATCH] Fix crash on attaching external threads

Fixes https://issues.dlang.org/show_bug.cgi?id=19313

Issue comes from combination of two factors:

1) Calling `thread_attachThis` does `new Thread` as part of its
   implementation (because any thread context has to be wrapped into
   Thread class for registration in runtime).
2) Current GC implementation is not prepared to be called from external
   thread context and will crash in several places if `Thread.getThis`
   returns `null`.

There are different possible fixes but it seems that the least intrusive
one is to simply skip collection when called from the context of
unregistered thread. Worst thing it can possibly do for any other code
not affected by the issue is to delay garbage collection a bit.
---
 src/gc/impl/conservative/gc.d      |  8 ++++++++
 test/thread/Makefile               |  7 ++++++-
 test/thread/src/external_threads.d | 22 ++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 test/thread/src/external_threads.d

diff --git a/src/gc/impl/conservative/gc.d b/src/gc/impl/conservative/gc.d
index e3e3b6da37..fa1d900808 100644
--- a/src/gc/impl/conservative/gc.d
+++ b/src/gc/impl/conservative/gc.d
@@ -2384,6 +2384,14 @@ struct Gcx
      */
     size_t fullcollect(bool nostack = false) nothrow
     {
+        // It is possible that `fullcollect` will be called from a thread which
+        // is not yet registered in runtime (because allocating `new Thread` is
+        // part of `thread_attachThis` implementation). In that case it is
+        // better not to try actually collecting anything
+
+        if (Thread.getThis() is null)
+            return 0;
+
         MonoTime start, stop, begin;
 
         if (config.profile)
diff --git a/test/thread/Makefile b/test/thread/Makefile
index ca27d1fb86..ba0eafc2e5 100644
--- a/test/thread/Makefile
+++ b/test/thread/Makefile
@@ -1,6 +1,6 @@
 include ../common.mak
 
-TESTS:=fiber_guard_page
+TESTS:=fiber_guard_page external_threads
 
 .PHONY: all clean
 all: $(addprefix $(ROOT)/,$(addsuffix .done,$(TESTS)))
@@ -11,6 +11,11 @@ $(ROOT)/fiber_guard_page.done: $(ROOT)/%.done : $(ROOT)/%
 	$(QUIET)$(TIMELIMIT)$(ROOT)/$* $(RUN_ARGS); rc=$$?; [ $$rc -eq 139 ] || [ $$rc -eq 138 ]
 	@touch $@
 
+$(ROOT)/external_threads.done: $(ROOT)/%.done : $(ROOT)/%
+	@echo Testing $*
+	$(QUIET)$(TIMELIMIT)$(ROOT)/$*
+	@touch $@
+
 $(ROOT)/%: $(SRC)/%.d
 	$(QUIET)$(DMD) $(DFLAGS) -of$@ $<
 
diff --git a/test/thread/src/external_threads.d b/test/thread/src/external_threads.d
new file mode 100644
index 0000000000..087c4bfebd
--- /dev/null
+++ b/test/thread/src/external_threads.d
@@ -0,0 +1,22 @@
+import core.sys.posix.pthread;
+import core.memory;
+
+extern(C)
+void* entry_point(void*)
+{
+    // try collecting - GC must ignore this call because this thread
+    // is not registered in runtime
+    GC.collect();
+    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);
+}