Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Fix issue 17413: Prevent deadlock in the GC init #1872

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix issue 17413: Prevent deadlock in the GC init #1872

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1e75c50
Make thread_attachThis nothrow
Burgos Jul 13, 2017
4c97b07
Enable GC in thread_attachThis in scope(success) instead scope(exit)
Burgos Jul 13, 2017
d5088da
Rename ConservativeGC._in_Finalizer to _is_Locked
Burgos Jul 13, 2017
8a61b43
Fix issue 17413: Prevent deadlock in the GC init
Burgos Jul 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/core/thread.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2073,9 +2073,9 @@ extern (C) bool thread_isMainThread() nothrow @nogc
*
* extern (C) void rt_moduleTlsCtor();
*/
extern (C) Thread thread_attachThis()
extern (C) Thread thread_attachThis() nothrow
{
GC.disable(); scope(exit) GC.enable();
GC.disable(); scope(success) GC.enable();

if (auto t = Thread.getThis())
return t;
Expand Down
20 changes: 10 additions & 10 deletions src/gc/impl/conservative/gc.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,12 @@ class ConservativeGC : GC

import core.internal.spinlock;
static gcLock = shared(AlignedSpinLock)(SpinLock.Contention.lengthy);
static bool _inFinalizer;
static bool _isLocked;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's easily confusable with the global GC lock? Also it's purpose is not locking but detecting reentrancy from finalizers. I think the name is rather appropriate.


// lock GC, throw InvalidMemoryOperationError on recursive locking during finalization
static void lockNR() @nogc nothrow
{
if (_inFinalizer)
if (_isLocked)
onInvalidMemoryOperationError();
gcLock.lock();
}
Expand Down Expand Up @@ -817,7 +817,7 @@ class ConservativeGC : GC

void free(void *p) nothrow
{
if (!p || _inFinalizer)
if (!p || _isLocked)
{
return;
}
Expand Down Expand Up @@ -1120,7 +1120,7 @@ class ConservativeGC : GC

bool inFinalizer() nothrow
{
return _inFinalizer;
return _isLocked;
}


Expand Down Expand Up @@ -1500,8 +1500,8 @@ struct Gcx
*/
void runFinalizers(in void[] segment) nothrow
{
ConservativeGC._inFinalizer = true;
scope (failure) ConservativeGC._inFinalizer = false;
ConservativeGC._isLocked = true;
scope (failure) ConservativeGC._isLocked = false;

foreach (pool; pooltable[0 .. npools])
{
Expand All @@ -1518,7 +1518,7 @@ struct Gcx
spool.runFinalizers(segment);
}
}
ConservativeGC._inFinalizer = false;
ConservativeGC._isLocked = false;
}

Pool* findPool(void* p) pure nothrow
Expand Down Expand Up @@ -2408,12 +2408,12 @@ struct Gcx
start = stop;
}

ConservativeGC._inFinalizer = true;
ConservativeGC._isLocked = true;
size_t freedLargePages=void;
{
scope (failure) ConservativeGC._inFinalizer = false;
scope (failure) ConservativeGC._isLocked = false;
freedLargePages = sweep();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inFinalizer is set around sweep b/c that's where we call object finalizers.

ConservativeGC._inFinalizer = false;
ConservativeGC._isLocked = false;
}

if (config.profile)
Expand Down