fix(coordinator): give each tab its own row count task so one cannot orphan another - #2062
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
This was referenced Aug 10, 2026
Merged
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.
Closes #2059.
currentRowCountTaskwas one slot on the coordinator shared by every tab in the window, while the work it holds is per tab. Two writers assigned to it without cancelling what was there, andteardown()never cancelled it at all.What went wrong
Tab A starts a row count. The user switches to tab B and runs a query there. Tab B's count overwrites the slot, dropping the only reference to tab A's task. Nothing can cancel it after that: not Stop, not a refresh, not closing the window. It runs to the end of its driver round trip, and because both task bodies capture
parentstrongly, it holds the whole coordinator alive while it does.teardown()cancels seven other handles and misses this one, so closing a window leaves every in-flight count running.What this changes
The handle becomes
rowCountTasks: [UUID: Task<Void, Never>], keyed by tab, which is howtableLoadTasksalready models the same shape. Four helpers own the slot so no call site can get it wrong:setRowCountTask(_:for:)cancels the tab's previous count before storing the new oneclearRowCountTask(for:)drops a finished task's handle without cancellingcancelRowCountTask(for:)for a single tabcancelAllRowCountTasks()for Stop and teardownCancel-before-assign alone would have been wrong here. With one shared slot, cancelling the predecessor means starting a count on tab B kills tab A's legitimate count. Keying by tab fixes the orphaning and the cross-tab interference together.
Also wired in:
teardown()now cancels every row count.supersedeExecution(for:)cancels that tab's count. Pointing a tab at another table makes its old count meaningless, and this stops the server working on it.changeManagerUpdateTask. It was declared and cancelled inteardown()but never assigned anywhere in the repo.Tests
RowCountTaskLifecycleTestscovers the six cases: a second count on the same tab cancels the first, a count on another tab does not, retargeting cancels only its own, teardown and Stop cancel all, and clearing a finished handle cancels nothing. The two existing tests inMainContentCoordinatorRefreshTeststhat poked the old slot are updated in the same commit.28 tests pass across
RowCountTaskLifecycleTests,MainContentCoordinatorRefreshTests,TabRetargetInvalidationTestsandDriverCancellationPolicyTests.swiftlint lint --strictreports 0 violations in 1300 files.Scope note
The write-back guards were already correct, so no late count could corrupt the grid. This is about work that outlives its owner, not wrong data.