Restore ordered delivery of repository and registry delegate callbacks - #10408
Merged
Conversation
plemarquand
requested review from
bkhouri,
bripeticca,
cmcgee1024,
daveinglis,
daveyc123,
dschaefer2,
jakepetroules,
owenv and
rconnell9
as code owners
August 13, 2026 16:38
plemarquand
force-pushed
the
serial-delegate-calls
branch
2 times, most recently
from
August 13, 2026 17:33
57f2fc1 to
61873c3
Compare
Add `SerialEventQueue` to Basics, basically an AsyncStream that one task
drains in order, holding the delegate it delivers to. `emit` returns as soon
as the event is queued, so it doesn't block the caller, and delivery is
serialized so callbacks arrive in the order they were emitted.
Both managers hold one directly now and emit through it, which replaces the
two `DelegateProxy` types (pass-throughs restating every protocol method by
that point).
Callbacks are now delivered in emission order. The tradeoff is that a slow
delegate delays the callbacks queued behind it, which the old
`.sharedConcurrent` dispatch didn't do; thats the cost of ordering them, and
these delegates just format a string and write a line so it shouldn't matter
in practice.
Also stops allocating when theres no delegate. `Task { await delegate?.foo() }`
allocated and enqueued a task even when delegate was nil, which the registry
progress handler did once per chunk received. `delegate?.emit` bails out first.
plemarquand
force-pushed
the
serial-delegate-calls
branch
from
August 13, 2026 17:34
61873c3 to
f635f8e
Compare
Contributor
Author
|
@swift-ci test |
Contributor
Author
|
@swift-ci test windows platform |
| let (stream, continuation) = AsyncStream.makeStream(of: Event.self, bufferingPolicy: .unbounded) | ||
| self.events = continuation | ||
|
|
||
| Task { |
Contributor
There was a problem hiding this comment.
This Task should be saved as an ivar and cancelled on deinit. Also, it'd be ideal if you could provide a function that allows the caller for wait for the task to finish.
plemarquand
force-pushed
the
serial-delegate-calls
branch
from
August 14, 2026 20:41
f643680 to
dc0e65b
Compare
Contributor
Author
|
@swift-ci test |
Contributor
Author
|
@swift-ci test windows |
1 similar comment
Contributor
Author
|
@swift-ci test windows |
Contributor
Author
|
@swift-ci test windows platform |
1 similar comment
Contributor
Author
|
@swift-ci test windows platform |
jakepetroules
approved these changes
Aug 17, 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
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.
Motivation:
RepositoryManagerandRegistryDownloadsManagerfired each delegate callback from its own unstructured Task:Two independently created tasks start in whatever order the executor picks, so nothing stopped
didFetchstarting beforewillFetch. The actor proxy they awaited serializes the calls it receives, but it can't do anything about the order they arrive in.Before #8721 these went through
delegateQueue.async { ... }and every caller passed.sharedConcurrent. A concurrent queue dequeues FIFO, so delivery started in order while the callbacks still ran independently. Moving to Task dropped the ordering and didn't put anything back.Modifications:
Add
SerialEventQueueto Basics, basically an AsyncStream that a single task drains, holding the delegate it delivers to.emityields into the stream and returns, so it doesn't block the caller, and the drain task calls the delegate one event at a time in the order they were emitted.Both managers hold one directly now and emit through it. That replaces the two
DelegateProxytypes (which had turned into pass-throughs restating every protocol method).Result:
Callbacks are delivered in the order they were emitted, so
didFetchcan no longer land beforewillFetch.The tradeoff is that a slow delegate now delays the callbacks queued behind it, which the old
.sharedConcurrentdispatch didn't do. Ordering callbacks means waiting on them, so I dont think theres a way around that short of a queue per package. These delegates format a string and write a line, so realistically it shouldn't matter.Also stops allocating when theres no delegate.
Task { await delegate?.foo() }allocated and enqueued a task even when delegate was nil, which the registry progress handler did once per chunk received.