Fix thread safety of disposableObjects in ServiceContainer.Dispose#611
Merged
Conversation
TrackInstance and Dispose both accessed the disposableObjects list without synchronization, causing "collection was modified" errors under concurrent load. - Lock disposableObjects.Add in TrackInstance using the existing lockObject - Atomically snapshot-and-clear disposableObjects under lock in Dispose, then dispose the snapshot outside the lock to avoid holding the lock during user code - Use a reference-equality HashSet to skip duplicates, consistent with Scope.Dispose - Add DisposableObjectComparer nested class for stable identity-based hashing - Add concurrent stress test (1000 iterations, 3 threads) as regression guard Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cover the duplicate-detection continue branches in Scope.DisposeAsync() (fast IAsyncDisposable path, IDisposable path, and slow async Await path) and the DisposableObjectComparer.Equals path in ServiceContainer.Dispose(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TrackInstanceandDispose()both accesseddisposableObjects(List<IDisposable>) without any synchronization, creating a data race under concurrent load — most visibly when services are still being first-resolved on background threads while the container is being disposed. Depending on .NET version and timing, this manifests asInvalidOperationException: Collection was modifiedorArgumentException: Destination array was not long enough.Dispose()also had no guard against the same instance being disposed more than once (e.g. if two container lifetimes happened to return the same object).Changes
TrackInstance— wrapsdisposableObjects.Add()inlock(container.lockObject), matching the locking pattern already used byScope.TrackInstance.ServiceContainer.Dispose()— atomically snapshots-and-clearsdisposableObjectsunderlockObject, then disposes the snapshot outside the lock so no lock is held during userDispose()calls (avoids deadlocks). A reference-equalityHashSetskips duplicate entries, consistent with howScope.Dispose()already works.DisposableObjectComparer— new private nested class usingRuntimeHelpers.GetHashCodefor stable object-identity hashing.Dispose_ConcurrentWithServiceCreation_DoesNotThrow— stress test with 3 concurrent threads (2 resolving services, 1 disposing) over 1000 iterations as a regression guard.Test plan
🤖 Generated with Claude Code