Fix: Async resolving the shared instances (avoiding duplicates) (3/3)#24
Merged
DanielCech merged 21 commits intomasterfrom Mar 6, 2026
Merged
Fix: Async resolving the shared instances (avoiding duplicates) (3/3)#24DanielCech merged 21 commits intomasterfrom
DanielCech merged 21 commits intomasterfrom
Conversation
DanielCech
changed the title
DanielCech
changed the title
TParizek
previously approved these changes
Feb 17, 2026
Contributor
TParizek
left a comment
TParizek
left a comment
There was a problem hiding this comment.
Awesome fix for that concurrency issue! It’s great to see us taking full advantage of Swift Structured Concurrency here.
# Conflicts: # Sources/Container/Async/AsyncContainer.swift
Generated by 🚫 Danger |
DanielCech
dismissed stale reviews from TParizek and robha141
The base branch was changed.
TParizek
approved these changes
Mar 6, 2026
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
3 participants
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.
This PR is a bit unexpected. I implemented some async tests, and especially tests for async resolving of the group of shared dependencies (concurrentResolveSharedDependency) were failing. It is not a very common case that we need to resolve so many same shared dependencies at the same time, but it is worth fixing.
Problem:
The test was failing because multiple concurrent resolves were all creating their own shared instance.
In the original AsyncContainer logic:
getDependencycheckssharedInstances. If missing, it awaits the factory. After await, it stores the shared instance. When several tasks hit this at once, they all pass the “no instance yet” check, and each one builds its own instance before any of them stores it.Solution
Added a task cache for shared resolutions in AsyncContainer, so it resolves await the same Task instead of creating duplicates.
The shared instance is now stored once the task completes, and the cache entry is cleared (also on errors).
This ensures all concurrent shared resolves return the same instance.
This PR is built on top of #23