ReviewOS

also looking at this

craft-native/craft

fix(ios): settle a Siri shortcut removal whose completion never comes

#227
Merged glennmichael123 wants to merge fix/ios-siri-removal-deadline into main
7 files +503 -94
9 days ago

Closes #211.

What was wrong

removeSiriShortcut answered only from inside the completion handler of +[NSUserActivity deleteSavedUserActivitiesWithPersistentIdentifiers:completionHandler:], on both runtimes. Nothing else ever settled the call. When the completion did not come, the page's promise waited for ever and the Zig reply slot stayed held. That happened twice on CI: runs 35098893353 and 35100931620, where the zig-slice fixture waited 900 s for i=38.

What it does now

Both runtimes give the call a 15 s deadline. If the completion has not arrived by then, the call rejects with TIMEOUT, and a completion that arrives later is logged and ignored. The deadline never answers removed: true: the deletion may still have happened, and removing again is harmless.

Why 15 s: craft.siri.remove arms no timeout of its own, so this deadline is the only thing that settles it; for _invoke and craft-bridge.js it is well under their 30 s, so the page hears the native answer rather than its own. It is also the budget the slice fixture already gives a daemon-served location fix on the same simulator. A completion that does arrive logs how long it took, so the number can be checked against CI.

Zig: why location's timeout could not just be copied

getCurrentPosition already has a deadline, and its timer moves into ios_async as scheduleDeadline / ticketFromDeadline so both callers share it. Location's behaviour does not change.

Siri's completion blocks are the problem. A void (^)(void) global block knows only its own index, and there was one block per reply slot. With a deadline, that goes wrong like this:

  1. The deadline answers removal A and releases slot k.
  2. Removal B leases slot k, because acquire hands out the lowest free slot, and gets the same block.
  3. A's late completion fires that block and answers B with removed: true before B's deletion has finished.

So blocks are now separate from reply slots: a table of 32 with the states free, waiting and owed.

  • Who answers. The deadline takes a waiting block only if the stored ticket matches on index and generation, and leaves it owed. The completion takes whatever its block holds. Each block leaves waiting exactly once, under the lock, so exactly one of them calls deliver*.
  • An owed block. It is never given to another call until its completion arrives. If every block is owed, the removal is refused with NATIVE_CALL_FAILED and a log line, rather than reusing one.
  • Order of the steps. Every fallible step, including the selector lookup that used to run after the call was parked, now happens before the slot is leased. The one exception is claiming a block, and its refusal is undone by the existing errdefers. Nothing can fail between parking the reply and handing Foundation the block.

ios_async also re-checks nothing when its main-queue hop runs, which is why the lock above decides who answers. That gap is filed as #225.

Swift

A [UUID: DispatchWorkItem] holds one entry per call, keyed by a token rather than callbackId, which can be nil and restarts on reload. The deadline and the completion both run on the main queue and remove the same entry, so only the one that finds it answers.

The completion now hops to the main queue before replying. Its queue is undocumented, and a zig: hand-off reply calls evaluateJavaScript with no hop of its own.

The fixture, and what CI's simulator actually does

page.m used to open Settings, which backgrounds the app, as soon as speech ended. The removal was often still waiting at that point, and a suspended app runs neither a framework completion nor a main-queue timer. So the fixture now opens Settings only once the removal has settled, one way or the other, and the harness waits for 54 explicitly.

Before choosing what i=38 should require, I measured what CI's simulator does, across three pushes of this branch:

runwhat the fixture did after the callcompletion
1held the app in the foreground until the 15 s deadlinenone
2held it in the foreground for 60 s morenone, 75 s after the call
3backgrounded it right after the deadline and waited 30 snone

Locally the same completion arrives 33–47 ms after the call. On main, the runs that passed saw i=38 only right after openSettings backgrounded the app while the call was still waiting. The two that failed (35098893353, 35100931620) were that same race lost.

So the completion's arrival on CI is the simulator's behaviour, not something the app or the fixture can make deterministic. The fixture now asserts what #211 is about, which the app does control: the call settles.

markermeansresult
38the completion delivered {removed: true, action: 'sliceAct'}ok
84the native deadline answered TIMEOUT instead of the page waiting for everok, with a note and a GitHub ::warning:: saying the completion path went unexercised, or that the completion came late and was ignored
86settled with anything elseFAIL
more than one of themsettled more than one wayFAIL, but only seen when both land before the app is backgrounded. The Zig host tests are what guard exactly one answer
nonenothing settled itthe harness times out, as before

The trade-off: on a CI run where the simulator never calls the completion, a regression in the completion block itself would show up as a warning, not a red job. It stays covered in three places:

  • the block-table host tests;
  • the global-block construction test;
  • every run where the simulator does call the completion, as it does locally, which still goes through i=38.

Tests that fail without the fix

  • bridge_mobile_siri.zig, new host tests on the block table:

    • a deadline answers a waiting removal once and leaves its block owed;
    • a completion after its deadline is ignored and frees the block;
    • a deadline that fires after the completion does nothing;
    • a stale deadline cannot expire a newer removal on the same reply slot;
    • a reply slot reused after a deadline never gets the block still owed;
    • with every block owed, a removal is refused;
    • the deadline is shorter than the page's own request timeout.

    Two mutations were run against them. Letting claimBlock reuse an owed block fails the reuse test and the refusal test. Matching the deadline on the slot index alone fails the stale-deadline test.

  • ios_async.zig: a deadline's context carries the whole ticket. This test moved here from location.

  • packages/ios/src/index.test.ts: the generated Swift schedules the deadline, rejects with TIMEOUT, takes the same entry on both paths, and never resolves removed: true from the deadline. The Swift arm only runs in apps without Zig, so no simulator suite reaches it. CI's ios-native-templates job compiles it, and packages/ios/scripts/typecheck-template.sh passes locally with no new warnings.

The slice fixture, run locally:

# this branch
ok: argument-free completion block delivered its parked reply (i=38 seen 1x)
ok: openSettings replied a real boolean, not "granted"/"denied" (i=54 seen 1x)
PASS
info: removeSiriShortcut: the deletion completion arrived after 33 ms

# this branch, with the Foundation call removed so the completion never comes
warning: removeSiriShortcut: +[NSUserActivity deleteSavedUserActivitiesWithPersistentIdentifiers:completionHandler:] did not call its completion within 15000 ms; answering TIMEOUT
ok: removeSiriShortcut settled with TIMEOUT from its native deadline instead of hanging (i=84 seen 1x)
note: the simulator never called the NSUserActivity deletion completion, so this run did not exercise that path
::warning title=removeSiriShortcut completion::the simulator never called the NSUserActivity deletion completion, so this run did not exercise that path
PASS

The same missing completion before this change is what CI runs 35098893353 and 35100931620 show: 900 s of waiting, then "the app never got that far".

Filed along the way

  • #223: Zig actions with the same shape (getPendingNotifications, scheduleNotification, getHealthData, saveHealthWorkout, openSettings, openURL), and why each needs this PR's owed-block design before a deadline is safe.
  • #224: the Swift-only equivalents (checkPermission('notifications'), registerPush, getProducts, Live Activity update and end).
  • #225: ios_async's main-queue hop checks only that a slot is in use, not which lease.
  • #226: cb_<n> restarts on every page load, so an answer to a call made before a reload can settle a different call after it. A deadline makes that answer certain to arrive, which is why it is filed now.
  • A comment on #221: the Swift Siri arms never settle on a missing argument either.

Verification

bun run test, bun run typecheck, bun run verify:packages, Pickier, the ios/typescript/android package tests, the Swift template typecheck, and the slice fixture locally on an iOS 26.5 simulator.

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

Sign in to comment on this pull request.