Skip to content

Commit

Permalink
Fix roots mechanism to avoid deadlocks and asserts (#402)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
schveiguy authored Nov 28, 2024
1 parent 6da2a74 commit 708c8ba
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
8 changes: 8 additions & 0 deletions sdlib/d/gc/collector.d
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
33 changes: 29 additions & 4 deletions sdlib/d/gc/global.d
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
Expand Down Expand Up @@ -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!");

Expand Down

0 comments on commit 708c8ba

Please sign in to comment.