From 708c8baaf1f30a6701f2b46a81570a512b5bd173 Mon Sep 17 00:00:00 2001
From: Steven Schveighoffer <schveiguy@gmail.com>
Date: Wed, 27 Nov 2024 20:37:47 -0500
Subject: [PATCH] Fix roots mechanism to avoid deadlocks and asserts (#402)

When roots are removed, avoid using allocations.

This allows the removeRoots function to be used in finalizers, a common
place to use them, since this is where you may want to unpin memory, or
free externally allocated memory that you then want to unregister.

When roots are added, we should be OK, because you shouldn't be adding
roots in a finalizer.

The issue with allocating in a finalizer is that the current arena is
locked, and allocating may try to lock the arena again.
---
 sdlib/d/gc/collector.d |  8 ++++++++
 sdlib/d/gc/global.d    | 33 +++++++++++++++++++++++++++++----
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/sdlib/d/gc/collector.d b/sdlib/d/gc/collector.d
index 499a427b0..92f4807ca 100644
--- a/sdlib/d/gc/collector.d
+++ b/sdlib/d/gc/collector.d
@@ -74,6 +74,14 @@ private:
 		threadCache.flush();
 
 		collect(gcCycle);
+
+		/**
+		 * Removing roots cannot realloc while inside a finalizer,
+		 * because that could cause a deadlock. So we must periodically
+		 * minimize the roots array, never when inside the collect
+		 * phase.
+		 */
+		gState.minimizeRoots();
 	}
 
 	void prepareGCCycle() {
diff --git a/sdlib/d/gc/global.d b/sdlib/d/gc/global.d
index 7ce9938ce..82b6c015e 100644
--- a/sdlib/d/gc/global.d
+++ b/sdlib/d/gc/global.d
@@ -83,6 +83,22 @@ public:
 		(cast(GCState*) &this).scanRootsImpl(scan);
 	}
 
+	/**
+	 * Tidy up any root structure. This should be called periodically to
+	 * ensure the roots structure does not become too large. Should not be
+	 * called during collection.
+	 */
+	void minimizeRoots() shared {
+		import d.gc.thread;
+		enterBusyState();
+		scope(exit) exitBusyState();
+
+		mutex.lock();
+		scope(exit) mutex.unlock();
+
+		(cast(GCState*) &this).minimizeRootsImpl();
+	}
+
 private:
 	void addRootsImpl(const void[] range) {
 		assert(mutex.isHeld(), "Mutex not held!");
@@ -125,14 +141,23 @@ private:
 			roots[i] = roots[length];
 			roots[length] = [];
 
-			import d.gc.tcache;
-			auto newRoots =
-				threadCache.realloc(roots.ptr, length * void*[].sizeof, true);
-			roots = (cast(const(void*)[]*) newRoots)[0 .. length];
+			roots = roots[0 .. length];
+
 			break;
 		}
 	}
 
+	void minimizeRootsImpl() {
+		assert(mutex.isHeld(), "Mutex not held!");
+
+		auto length = roots.length;
+
+		import d.gc.tcache;
+		auto newRoots =
+			threadCache.realloc(roots.ptr, length * void*[].sizeof, true);
+		roots = (cast(const(void*)[]*) newRoots)[0 .. length];
+	}
+
 	void scanRootsImpl(ScanDg scan) {
 		assert(mutex.isHeld(), "Mutex not held!");