Skip to content

Commit fa451cf

Browse files
authored
ConditionalWeakTable - don't link Containers when no entries are transferred (#108941)
ConditionalWeakTable uses internal Container objects for the underlying table. Container entries are write-once because the read path is lock-free. When a Container is full, a new Container is allocated, entries are copied, and compaction can occur (if there aren't any currently live enumerators relying on specific indices). A two-pass finalization scheme is used to free the entries (dependent handles) and then the Containers themselves. Finalization provides a guarantee that the Container is no longer in use, and the second pass accounts for finalization resurrection. Because entries can be duplicated across Containers, each Container contains a link to the one that replaces it. This can _greatly_ extend the lifetime of Containers. (See #50683 and #108447.) However, if the Container is empty and not being enumerated, there is no need to link it to the next Container. This PR handles that case, which includes microbenchmarks where single entries are added and removed from ConditionalWeakTable and equivalent tests where TransactionScope is functioning as a wrapper around a ConditionalWeakTable entry. Of course, this is only a partial solution because a single live entry or enumerator leaves the old behavior. Another caveat is that the finalization queue can be filled faster than it can be emptied, though this is more likely in microbenchmarks where other work isn't being done.
1 parent 637e822 commit fa451cf

File tree

1 file changed

+14
-1
lines changed

1 file changed

+14
-1
lines changed

src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,10 +698,13 @@ internal Container Resize(int newSize)
698698
Entry[] newEntries = new Entry[newSize];
699699
int newEntriesIndex = 0;
700700
bool activeEnumerators = _parent != null && _parent._activeEnumeratorRefCount > 0;
701+
bool transferredHandles;
701702

702703
// Migrate existing entries to the new table.
703704
if (activeEnumerators)
704705
{
706+
transferredHandles = true;
707+
705708
// There's at least one active enumerator, which means we don't want to
706709
// remove any expired/removed entries, in order to not affect existing
707710
// entries indices. Copy over the entries while rebuilding the buckets list,
@@ -721,6 +724,8 @@ internal Container Resize(int newSize)
721724
}
722725
else
723726
{
727+
transferredHandles = false;
728+
724729
// There are no active enumerators, which means we want to compact by
725730
// removing expired/removed entries.
726731
for (int entriesIndex = 0; entriesIndex < _entries.Length; entriesIndex++)
@@ -732,6 +737,8 @@ internal Container Resize(int newSize)
732737
{
733738
if (depHnd.UnsafeGetTarget() is not null)
734739
{
740+
transferredHandles = true;
741+
735742
ref Entry newEntry = ref newEntries[newEntriesIndex];
736743

737744
// Entry is used and has not expired. Link it into the appropriate bucket list.
@@ -765,7 +772,13 @@ internal Container Resize(int newSize)
765772
// from being finalized, as it no longer has any responsibility for any cleanup.
766773
GC.SuppressFinalize(this);
767774
}
768-
_oldKeepAlive = newContainer; // once this is set, the old container's finalizer will not free transferred dependent handles
775+
776+
if (transferredHandles)
777+
{
778+
// Once this is set, the old container's finalizer will not free transferred dependent handles,
779+
// and the new container's finalizer can't be run until this container is no longer in use.
780+
_oldKeepAlive = newContainer;
781+
}
769782

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

0 commit comments

Comments
 (0)