From 2ed4a03511de49644d9cb17152685a05d2d91b58 Mon Sep 17 00:00:00 2001
From: Vladimir Panteleev <git@thecybershadow.net>
Date: Sun, 6 Oct 2019 11:54:46 +0000
Subject: [PATCH] Add fork handling to GC

Make sure a fork does not happen while GC code is running, thus
leaving it in an inconsistent state.

Does not yet fix Druntime's thread list after a fork, so garbage
collection inside the forked process will still fail.

Fixes issue #20271.
---
 src/gc/impl/conservative/gc.d | 67 ++++++++++++++++++++++++-----------
 test/gc/Makefile              |  5 ++-
 test/gc/forkgc2.d             | 22 ++++++++++++
 3 files changed, 73 insertions(+), 21 deletions(-)
 create mode 100644 test/gc/forkgc2.d

diff --git a/src/gc/impl/conservative/gc.d b/src/gc/impl/conservative/gc.d
index 6b0a8ccb70..45234a764d 100644
--- a/src/gc/impl/conservative/gc.d
+++ b/src/gc/impl/conservative/gc.d
@@ -1251,6 +1251,7 @@ struct Gcx
         alias leakDetector = LeakDetector;
 
     SmallObjectPool*[B_NUMSMALL] recoverPool;
+    version (Posix) __gshared Gcx* instance;
 
     void initialize()
     {
@@ -1262,10 +1263,23 @@ struct Gcx
         usedSmallPages = usedLargePages = 0;
         mappedPages = 0;
         //printf("gcx = %p, self = %x\n", &this, self);
+        version (Posix)
+        {
+            import core.sys.posix.pthread : pthread_atfork;
+            instance = &this;
+            __gshared atforkHandlersInstalled = false;
+            if (!atforkHandlersInstalled)
+            {
+                pthread_atfork(
+                    &_d_gcx_atfork_prepare,
+                    &_d_gcx_atfork_parent,
+                    &_d_gcx_atfork_child);
+                atforkHandlersInstalled = true;
+            }
+        }
         debug(INVARIANT) initialized = true;
     }
 
-
     void Dtor()
     {
         if (config.profile)
@@ -1310,6 +1324,8 @@ struct Gcx
                    pauseTime, maxPause, apitxt.ptr);
         }
 
+        version (Posix)
+            instance = null;
         version (COLLECT_PARALLEL)
             stopScanThreads();
 
@@ -2841,13 +2857,6 @@ struct Gcx
             sigmask_rc = pthread_sigmask(SIG_SETMASK, &old_mask, null);
             assert(sigmask_rc == 0, "failed to set up GC scan thread sigmask");
         }
-
-        version (Posix)
-        {
-            import core.sys.posix.pthread;
-            forkedGcx = &this;
-            pthread_atfork(null, null, &initChildAfterFork);
-        }
     }
 
     void stopScanThreads() nothrow
@@ -2883,8 +2892,6 @@ struct Gcx
         cstdlib.free(scanThreadData);
         // scanThreadData = null; // keep non-null to not start again after shutdown
         numScanThreads = 0;
-        version (Posix)
-            forkedGcx = null;
 
         debug(PARALLEL_PRINTF) printf("stopScanThreads done\n");
     }
@@ -2942,20 +2949,40 @@ struct Gcx
 
     version (Posix)
     {
-        // make sure the threads and event handles are reinitialized in a fork
-        __gshared Gcx* forkedGcx;
+        // A fork might happen while GC code is running in a different thread.
+        // Because that would leave the GC in an inconsistent state,
+        // make sure no GC code is running by acquiring the lock here,
+        // before a fork.
+
+        extern(C) static void _d_gcx_atfork_prepare()
+        {
+            if (instance)
+                ConservativeGC.lockNR();
+        }
+
+        extern(C) static void _d_gcx_atfork_parent()
+        {
+            if (instance)
+                ConservativeGC.gcLock.unlock();
+        }
 
-        extern(C) static void initChildAfterFork()
+        extern(C) static void _d_gcx_atfork_child()
         {
-            if (forkedGcx && forkedGcx.scanThreadData)
+            if (instance)
             {
-                cstdlib.free(forkedGcx.scanThreadData);
-                forkedGcx.numScanThreads = 0;
-                forkedGcx.scanThreadData = null;
-                forkedGcx.busyThreads = 0;
+                ConservativeGC.gcLock.unlock();
 
-                memset(&forkedGcx.evStart, 0, Event.sizeof);
-                memset(&forkedGcx.evDone, 0, Event.sizeof);
+                // make sure the threads and event handles are reinitialized in a fork
+                if (Gcx.instance.scanThreadData)
+                {
+                    cstdlib.free(Gcx.instance.scanThreadData);
+                    Gcx.instance.numScanThreads = 0;
+                    Gcx.instance.scanThreadData = null;
+                    Gcx.instance.busyThreads = 0;
+
+                    memset(&Gcx.instance.evStart, 0, Gcx.instance.evStart.sizeof);
+                    memset(&Gcx.instance.evDone, 0, Gcx.instance.evDone.sizeof);
+                }
             }
         }
     }
diff --git a/test/gc/Makefile b/test/gc/Makefile
index 2328754b61..7a81cfd49f 100644
--- a/test/gc/Makefile
+++ b/test/gc/Makefile
@@ -1,6 +1,6 @@
 include ../common.mak
 
-TESTS := sentinel printf memstomp invariant logging precise precisegc forkgc sigmaskgc
+TESTS := sentinel printf memstomp invariant logging precise precisegc forkgc forkgc2 sigmaskgc
 
 SRC_GC = ../../src/gc/impl/conservative/gc.d
 SRC = $(SRC_GC) ../../src/rt/lifetime.d
@@ -40,6 +40,9 @@ $(ROOT)/precisegc: $(SRC)
 $(ROOT)/forkgc: forkgc.d
 	$(DMD) $(UDFLAGS) -of$@ forkgc.d
 
+$(ROOT)/forkgc2: forkgc2.d
+	$(DMD) $(UDFLAGS) -of$@ forkgc2.d
+
 $(ROOT)/sigmaskgc: sigmaskgc.d
 	$(DMD) $(UDFLAGS) -of$@ sigmaskgc.d
 
diff --git a/test/gc/forkgc2.d b/test/gc/forkgc2.d
new file mode 100644
index 0000000000..de7796ced7
--- /dev/null
+++ b/test/gc/forkgc2.d
@@ -0,0 +1,22 @@
+import core.stdc.stdlib : exit;
+import core.sys.posix.sys.wait : waitpid;
+import core.sys.posix.unistd : fork;
+import core.thread : Thread;
+
+void main()
+{
+    foreach (t; 0 .. 10)
+        new Thread({
+            foreach (n; 0 .. 100)
+            {
+                foreach (x; 0 .. 100)
+                    new ubyte[x];
+                auto f = fork();
+                assert(f >= 0);
+                if (f == 0)
+                    exit(0);
+                else
+                    waitpid(f, null, 0);
+            }
+        }).start();
+}