Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConditionalWeakTable - don't link Containers when no entries are transferred #108941

Merged
merged 2 commits into from
Dec 6, 2024
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -698,10 +698,13 @@ internal Container Resize(int newSize)
Entry[] newEntries = new Entry[newSize];
int newEntriesIndex = 0;
bool activeEnumerators = _parent != null && _parent._activeEnumeratorRefCount > 0;
bool transferredHandles;

// Migrate existing entries to the new table.
if (activeEnumerators)
{
transferredHandles = true;

// There's at least one active enumerator, which means we don't want to
// remove any expired/removed entries, in order to not affect existing
// entries indices. Copy over the entries while rebuilding the buckets list,
Expand All @@ -721,6 +724,8 @@ internal Container Resize(int newSize)
}
else
{
transferredHandles = false;

// There are no active enumerators, which means we want to compact by
// removing expired/removed entries.
for (int entriesIndex = 0; entriesIndex < _entries.Length; entriesIndex++)
Expand All @@ -732,6 +737,8 @@ internal Container Resize(int newSize)
{
if (depHnd.UnsafeGetTarget() is not null)
{
transferredHandles = true;

ref Entry newEntry = ref newEntries[newEntriesIndex];

// Entry is used and has not expired. Link it into the appropriate bucket list.
Expand Down Expand Up @@ -765,7 +772,13 @@ internal Container Resize(int newSize)
// from being finalized, as it no longer has any responsibility for any cleanup.
GC.SuppressFinalize(this);
}
_oldKeepAlive = newContainer; // once this is set, the old container's finalizer will not free transferred dependent handles

if (transferredHandles)
{
// Once this is set, the old container's finalizer will not free transferred dependent handles,
// and the new container's finalizer can't be run until this container is no longer in use.
_oldKeepAlive = newContainer;
}

GC.KeepAlive(this); // ensure we don't get finalized while accessing DependentHandles.

Expand Down
Loading