ReviewOS

also looking at this

craft-native/craft

refactor(ios): lift #211's owed-block design out of Siri, so other actions can use it

#245
Merged glennmichael123 wants to merge fix/ios-pending-call-table into main
2 files +248 -195
9 days ago

Refs #223. Groundwork — no behaviour change, and it closes nothing on its own.

Why this is not just "call scheduleDeadline six times"

#223 lists six Zig actions with the shape #211 found in removeSiriShortcut: no person involved, the only answer a completion from a system daemon, and nothing else settling the call if it never arrives.

None of them can take a deadline as they stand. Each keeps a side table read by reply-slot index alone — takeCall, takeSettingsCall, takePendingCall, takeScheduleCall:

  1. the deadline answers and frees slot k;
  2. the next call leases slot k and overwrites the entry;
  3. the first call's late completion fires the same block, takes the new entry, and answers the second call with the first one's result.

Fixing #225 did not solve this. That fixed the pool's own main-queue hop, which now carries a whole ticket. These tables are the modules' own, and the block that fires still knows only its index.

What #211 did, and what this lifts

Siri's answer was to stop choosing blocks by reply slot. A block is its own resource, in one of three states — free, waiting a parked call, or owed because the deadline answered and the framework has not called back yet. An owed block is never handed to another call. There are twice as many blocks as reply slots, so every slot can be waiting while as many blocks again are still owed.

That is now ios_pending.Table(Call), generic over whatever a module needs to park: a shaped reply for Siri, a selector pair for notifications, a chain of stages for HealthKit. The table owns only who may answer, and when. The caller keeps the block array — its shape is the framework's, void (^)(void) here and void (^)(NSArray *) elsewhere — and owns whatever it parks; whichever side takes the entry frees it.

Three invariants, each with a test:

  • The deadline matches the whole ticket, index and generation. Matching the index alone is precisely the bug.
  • An expired block stays owed, so a later call cannot be given it.
  • Each block leaves waiting or owed exactly once, under the lock, so the completion and the deadline cannot both answer.

Siri is migrated, which is what proves the extraction

bridge_mobile_siri.zig keeps its behaviour and its wording; only the table moves. Four of its tests drove the table rather than the action, and those are replaced by seven in ios_pending.zig covering the same invariants plus a stray settle and the block/slot ratio. Two Siri-specific ones stay: that a refused removal leases no slot, and that this action declines when every block is owed rather than reusing one.

A canary confirmed the new tests actually run. A file can be imported without its tests being enrolled, in which case they pass by never executing — build.zig already carries a note about proving this wiring the same way. Inverting one assertion fails the suite, so they are real.

mutationfails
the deadline matches the slot index alone1
an expired block is freed instead of kept owed3
a late completion answers anyway1

What comes next

Each of the six actions, on this table, one or two per PR — they are separate modules with separate side tables and separate failure modes, and openSettings in particular needs care: the app is backgrounded while Settings is open, a main-queue deadline does not run while suspended, and a naive deadline would report TIMEOUT for an open that succeeded.

#223 also lists failures after the lease in the same files — openSettings looking up its selector after publishing the call, getHealthData resolving classes after publishCall, and a sel_registerName(...) orelse return; in the HealthKit chain that releases neither the parked reply nor the slot. Those belong with each action's own PR, where #211's "every fallible step above acquire" rule gets applied.

Verification

zig build test and zig build test:ios, Pickier (0 errors), and the ios/android package tests (153).

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

Sign in to comment on this pull request.