fix(coordinator): stop a stale waiter withdrawal cancelling the fetch that replaced it

[datlechin]

Found by a review pass over the batch merged in #2062, #2063, #2064 and #2065. This is a defect in #2063, which I wrote.

The bug

SchemaColumnStore shares one fetch between every caller asking for the same key, and counts live waiters so the fetch is only cancelled once the last of them has gone. The cleanup runs from withTaskCancellationHandler's onCancel, which is not main actor isolated, so it hops:

onCancel: { Task { @MainActor [weak self] in self?.withdrawWaiter(from: key) } }

That hop is deferred, and withdrawWaiter looked up loads[key] positionally with no check that the load it finds is the one the cancelled waiter joined. So:

1. W1 loads key K. loads[K] holds task T1, one waiter.
2. W1 is cancelled. The hop is scheduled, not yet run.
3. T1 finishes on its own, W1 resumes and its own cleanup removes loads[K].
4. A new caller W2 arrives, starts T2, loads[K] now holds T2 with one waiter.
5. The hop from step 2 finally runs, decrements W2's count to zero and cancels T2.

W2's fetch dies with nobody having abandoned it. In the app that is a table whose column details silently fail to load, so its default sort and hidden columns do not apply.

The same missing identity check let a cancelled-but-still-running fetch write its result over a newer one, because the write to entries[key] was unconditional.

The fix

Every Load carries an id. The three places that act on a load check it first:

• withdrawWaiter(from:loadID:) ignores a withdrawal aimed at a load the key no longer holds.
• load's completion cleanup only removes loads[key] if it still owns it.
• The fetch task only writes entries[key] if it still owns the key, so a superseded fetch cannot overwrite a newer result, and a fetch outstanding across removeAll() can no longer repopulate a cleared cache.

The separate generation counter that removeAll() used is gone: the per-load id subsumes it and covers the per-key races it never could.

Tests

Two added, both deterministic rather than timing dependent. The stale-withdrawal test drives the exact interleaving through a #if DEBUG seam (loadID(for:) and withdrawWaiterForTesting(from:loadID:)), which is the same shape as DatabaseManager.injectSession. Reproducing it by racing real hops would be flaky in both directions. The second test pins that a superseded fetch cannot overwrite a newer result.

28 tests pass across SchemaColumnStoreCancellationTests and SQLSchemaProviderTests. swiftlint lint --strict reports 0 violations in 1322 files.

No CHANGELOG entry: #2063 has not shipped yet, so per the repo's rule this folds into that entry rather than adding a "Fixed" line for something unreleased.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.