ReviewOS

also looking at this

craft-native/craft

fix(ios): give the Swift-only calls that wait on a framework callback a deadline

#244
Merged glennmichael123 wants to merge fix/ios-swift-deadlines into main
2 files +192 -11
10 days ago

Closes #224.

What was wrong

Five actions are answered only by Swift — on both runtimes, so no Zig deadline reaches them — and each one's only answer comes from a framework callback with no person in front of it:

actionwaits on
checkPermission('notifications')UNUserNotificationCenter.getNotificationSettings
registerPush, after the grantdidRegisterForRemoteNotificationsWithDeviceToken / didFail…
getProductsawait Product.products(for:)
updateLiveActivityawait activity.update
endLiveActivityawait activity.end

If the callback never came, nothing settled the page's promise. That is not hypothetical: #211 is the same shape, and CI's simulator declined to call an NSUserActivity deletion completion on three separate runs.

One mechanism, not five copies

let token = armDeadline(seconds, callbackId: callbackId, error: "…")
// …framework call…
DispatchQueue.main.async { [weak self] in
    guard let self, self.claimDeadline(token) else { return }
    self.resolveCallback(callbackId, result: …)
}

claimDeadline is what the callback and the deadline race for. Both run on the main queue, so the race is decided there rather than by a lock, and the loser finds nothing and does nothing. A deadline reports TIMEOUT and never that the work happened — it may well have, and saying otherwise would be a wrong answer reported as success.

Each callback hops to the main queue before claiming, because the queue a framework completion arrives on is its own business and a zig: hand-off reply reaches evaluateJavaScript with no hop of its own — the same reasoning #211 recorded.

The budgets

10s for the notification-settings read and the Live Activity operations, 20s for the APNs round trip and the StoreKit fetch — the two that reach the network. All comfortably under the 30s the page's own _invoke allows, so the caller hears the native answer rather than its own timeout.

registerPush had a second bug

pendingPushCallbackId = callbackId

A second registration overwrote the first's callbackId, and the first promise then never settled at all — deadline or no deadline. A displaced call is now rejected with CANCELLED, the way a replaced getCurrentPosition request is: being displaced is an answer the caller can act on, and silence is not.

Two details worth naming:

  • The deadline is armed after the grant, not at dispatch. Until the person answers the authorization prompt there is someone to wait for; only the APNs round trip afterwards is unattended. Timing out a prompt on someone's screen would be the wrong behaviour.
  • A token that arrives late still reaches the page. craftPushToken goes out whether or not the caller is still waiting — a page listening for the event is not the caller that timed out, and the token is still this app's.

What this does not do

Siri keeps pendingSiriRemovals, its own table from #211. armDeadline takes the whole error message precisely so moving it over is mechanical, but doing it here would mean rewriting assertions on already-shipped code for no behavioural gain.

The Zig half, #223, is a different problem and not in here: those modules keep side tables read by reply-slot index alone, so a deadline that frees a slot lets the next call lease it and the first call's late completion answers the second. That needs #211's owed-block design generalised first, which is its own piece of work.

Tests that fail without the fix

mutationfails
a second registerPush drops the first silently again1
the deadline resolves instead of reporting TIMEOUT1
StoreKit loses its own budget and borrows another1

The assertions are structural rather than textual where they can be — each budget is checked for its declaration and its use count, and armDeadline/claimDeadline for theirs, so a sixth site added without a deadline moves the count and fails. My first version pinned the indentation of a multi-line call, which broke on the site nested inside a switch — structural counts do not have that problem.

Verification

zig build test, bun run typecheck, bun run verify:packages, Pickier (0 errors), the ios/android/protocol tests (209), and packages/ios/scripts/typecheck-template.sh — the generated Swift typechecks with no new warnings.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.