MOB-133: grow the tap tables on demand instead of capping at 256 - #119
Conversation
The tap registry was a fixed TapHandle[2][256], and the handle encoding packed 8 slot bits into a positive int32 alongside 23 generation bits. Anything past slot 255 got the -1 "no handler" sentinel. That is not graceful degradation. The element still renders, still looks tappable, and does nothing — no error, no crash, nothing in the UI to suggest the tap was never wired up. On the benchmark screen it was 359 of 615 interactive elements: a 200-row list where more than half the buttons, fields and toggles were inert. The earlier half of this issue fixed the reporting (359 synchronous log writes per frame, 13ms of a 27ms frame); it did not make the elements work. Two changes, which have to happen together. Slots get 12 bits instead of 8 — 4096 instead of 256 — and the generation drops from 23 bits to 19. That trade is the cost of the split: at 60fps, 2^19 frames is about 2.4 hours before the generation wraps, and generationAge is modular so a wrap is handled rather than merely survived. A handle is only meaningful for the frame it was minted in, so hours of headroom is ample. And the tables are allocated to fit rather than declared at the ceiling. A fixed 4096-entry pair would be ~700KB resident in every app, nearly all of which register a few dozen handles. They start at 256 — the old size, so nobody pays more than before — and double on demand. Growth is safe because of an invariant that already held: every reader resolves under tap_mutex, and the pointers those resolvers return are used before it is released. Growth takes the same lock. Only the table being BUILT is grown mid-frame; the active one is untouched until the swap. One ordering detail matters: register_tap grows before it caches `tap_tables[1 - tap_active]`. Caching first would be a use-after-realloc. Verified end to end on both platforms — not "registers without complaint" but "responds when tapped": Android (Moto G Power): 610 handles at 200 rows, 1510 at 500, zero exhaustion where there were 359 per frame. Scrolled to row #188 (slot ~564) and tapped twice; the tap counter went 0 -> 1 -> 2. iOS (simulator): 615 and 1515 handles, zero exhaustion. Scrolled to row #92 (slot ~283) and tapped twice; the header read t/n/g=2/0/0. The tap matters, not just the count: Mob.RenderStats's `taps` counts handle-valued props and -1 is an integer, so it read 615 before this fix too. The exhaustion count is the honest signal for registration; only a tap proves the handle resolves. Three new codec tests: the bit split sums to 31 and every slot round-trips under the lowest and highest legal generations with the handle staying positive (a negative handle would collide with the -1 sentinel); a slot past the limit is refused rather than wrapping onto slot 0; and an old 8-bit-split handle does not decode to a live pair. No version bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ruption The review's verdict was "safe to merge", but it proved with ASan that this change upgrades a known-benign bug into a memory-corruption primitive. That is worth fixing before merge rather than after. ## The corruption path Only clear_taps resets tap_build_count, so a set_root arriving without an intervening clear_taps carries the previous frame's count. When both tables were a fixed 256 the worst case was committing the wrong handlers — a correctness bug, and the one Mob.Sender's own moduledoc already describes for two screens racing clear/register/set_root. Now the tables are separate heap allocations that can differ in size, so the carryover loop reads past the end of whichever is smaller, and once tap_handle_next is set from the stale count the throttle path WRITES past it too. The reviewer reproduced both a heap-buffer-overflow and a NULL deref from transcribed-verbatim logic. Both platforms now clamp the commit to the capacity of the table being published, guard the carryover by the previous table's capacity, and reset tap_build_count in set_root so a second set_root commits an empty table rather than re-committing against whatever the other table holds. iOS also gains the null guards Zig already had — the two were not equally defensive against the same input. ## A deadlock landmine Zig's snapChangeTap unlocks explicitly on every path, so the `orelse return null` I added would have left tap_mutex held forever, freezing every subsequent tap, gesture and render. Unreachable today — tap_active_count is only set from a build count a successful tapGrowLocked produced — so it is now an explicit unwrap that documents the invariant instead of a silent leak. ## Claims corrected The decision record said generationAge "is modular so a wrap is handled rather than merely survived". True of the arithmetic, and it does not address the alias: after a full cycle a stale handle is numerically identical to a fresh one, and slotForActive cannot tell them apart. The window shrank 16x with this change (38.8h -> 2.4h at 60fps). Now stated plainly, including what keeps it theoretical, rather than implied to be covered. guides/components.md still told users the cap was 256 fixed-size pools — the doc someone reads when deciding whether their list needs virtualising. ## Tests for the things the design depends on Three added, none of which existed: - the two codecs agree on the bit split. iOS hard-codes 4096, << 12, 0xfff and 0x7ffff by hand while Android derives everything from slot_bits; they are independent implementations of one wire format and drift routes events to the wrong process. - register_tap grows before caching a pointer into the table. The decision record calls this out as "easy to get wrong" and nothing guarded it. - set_root commits no more slots than the table holds, and resets the count. An existing test pinned the literal `tap_active_count = tap_build_count`, which the clamp replaces; updated to the new expression, with the property it exists for (all three commit under one lock) unchanged. Re-verified on the Moto G after the clamp, since it changes what set_root commits: 810 handles at 200 rows and 2010 at 500, zero exhaustion, and a tap on row #194 (slot ~775) plus a long press on the same row both register. 1383 tests, credo clean, zig/clang formatters clean, codec 8/8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GenericJam
commented
Sep 2, 2026
Adversarial review appliedVerdict was "safe to merge as-is", but the review proved with ASan that this change upgrades a known-benign race into a memory-corruption primitive. Fixed before merge rather than after. The corruption path. Only A deadlock landmine. Zig's Claims corrected. The decision record said Three tests added, none of which existed: the two codecs agree on the bit split (iOS hard-codes Re-verified on the Moto G after the clamp, since it changes what 1383 tests, credo clean, formatters clean, codec 8/8. CI green. Deferred, tracked separately: |
Uh oh!
There was an error while loading. Please reload this page.
The half of MOB-133 that was left open. No version bump.
The bug
The tap registry was a fixed
TapHandle tap_tables[2][256], and the handle encoding packed 8 slot bits into a positiveint32alongside 23 generation bits. Anything past slot 255 got the-1"no handler" sentinel fromregister_tap.That is not graceful degradation. The element still renders, still looks tappable, and does nothing — no error, no crash, nothing in the UI to suggest the tap was never wired up. On the benchmark screen it was 359 of 615 interactive elements: a 200-row list where more than half the buttons, fields and toggles were inert.
The earlier half of this issue fixed the reporting (359 synchronous log writes per frame, 13 ms of a 27 ms frame). It did not make the elements work.
The change
Slots get 12 bits instead of 8 — 4096 instead of 256 — and the generation drops from 23 bits to 19. That is the cost of the split: at 60 fps, 2^19 frames is ~2.4 hours before the generation wraps, and
generationAgeis already modular so a wrap is handled rather than merely survived. A handle is only meaningful for the frame it was minted in, so hours of headroom is ample.The tables are allocated to fit rather than declared at the ceiling. A fixed 4096-entry pair would be ~700 KB resident in every app, nearly all of which register a few dozen handles. They start at 256 — the old size, so nobody pays more than before — and double on demand.
Growth is safe because of an invariant that already held: every reader resolves under
tap_mutex, and the pointers those resolvers return are used before it is released. Growth takes the same lock. Only the table being built is grown mid-frame; the active one is untouched until the swap inset_root.One ordering detail matters:
register_tapgrows before it cachestap_tables[1 - tap_active]. Caching first would be a use-after-realloc.Verified end to end on both platforms
Not "registers without complaint" but "responds when tapped":
t/n/g=2/0/0The tap matters, not just the count:
Mob.RenderStats'stapscounts handle-valued props and-1is an integer, so it read 615 before this fix too. The exhaustion count is the honest signal for registration; only an actual tap proves the handle resolves.Tests
Three new codec tests (8 total, all passing):
-1sentinel and make a live element look inert1380 Elixir tests pass,
mix credo --strictclean,zig fmt/clang-formatclean.Consequences
reallocmeans allocation failure is now possible where it was not. It is treated exactly like exhaustion: the sentinel, and the report.