docs: correct three stale rows in the mobile surface matrix - #92
Merged
Conversation
The matrix had drifted in both directions — overstating something broken and understating something shipped, twice. - **Notification tap handling: ✅ → 🟡.** It claimed "Foreground + background + cold-start". Cold-start does not work: the payload is handed to mob_set_launch_notification before nif_load has created the mutex, so the setter returns early and take_launch_notification/0 gets nil. On Android the host calls it ~75 lines before the BEAM thread is even created, so the drop is guaranteed, not racy; on iOS the integration point mob_beam.h documents (didFinishLaunchingWithOptions:) is likewise pre-BEAM. Points at #81 and the community fix in #77, which will flip this back to ✅ when it lands. - **`<Modal>` (sheet presentation): 🟡 → ✅.** It said "full sheet-style modal is plugin territory". Mob.UI.sheet/2 has been core since 0.7.29 (iOS .sheet), with the Android Material 3 ModalBottomSheet renderer published in mob_new 0.4.24 — noted, since Android needs a project generated by that version or newer. - **Bottom sheets: ❌ → ✅.** Listed as a plugin candidate; it is literally the same primitive, with :detents. Notes the documented iOS scrim-opacity limitation rather than implying parity. Verified rather than assumed: Mob.UI.sheet/2 at lib/mob/ui.ex:336, MobSheetView in ios/MobRootView.swift, and MobSheet in the published mob_new 0.4.24 template. Swept the neighbouring rows too — Bluetooth Classic and QR/barcode are accurate. Separately, mob_video (0.1.0), mob_touch (0.1.0) and mob_screencast (0.1.1) are published on Hex but have no rows at all, so the matrix reads as though those capabilities don't exist. Left out of this commit rather than guessed at; each needs its platform support checked before it earns a row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 30, 2026
GenericJam added a commit
that referenced
this pull request
Sep 2, 2026
* MOB-133: grow the tap tables on demand instead of capping at 256 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> * MOB-133: act on review — stop the growth turning a race into heap corruption 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> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The matrix had drifted in both directions — overstating something broken, and understating something shipped, twice.
<Modal>(sheet presentation)Mob.UI.sheet/2shipped:detentsNotification tap handling — the overstatement
It claimed "Foreground + background + cold-start". Cold-start does not work: the payload is handed to
mob_set_launch_notificationbeforenif_loadhas created the mutex, so the setter returns early andtake_launch_notification/0getsnil.On Android the host calls it ~75 lines before the BEAM thread is even created, so the drop is guaranteed rather than racy. On iOS the integration point
mob_beam.hitself documents (didFinishLaunchingWithOptions:) is likewise pre-BEAM.Points at #81 and @asheehan's fix in #77 — landing that flips this row back to ✅.
Sheets — the understatements
<Modal>said "full sheet-style modal is plugin territory", andBottom sheetswas listed as a plugin candidate. Both are wrong as of this week:Mob.UI.sheet/2has been core since 0.7.29 (iOS.sheet), with the Android Material 3ModalBottomSheetrenderer published inmob_new0.4.24.Verified rather than assumed —
lib/mob/ui.ex:336,MobSheetViewinios/MobRootView.swift, andMobSheetin the publishedmob_new0.4.24 template. The Android row notes themob_new≥ 0.4.24 requirement, and the bottom-sheet row notes the documented iOS scrim-opacity limitation rather than implying parity.Also swept, not fixed here
Bluetooth Classic and QR/barcode rows check out. But
mob_video(0.1.0),mob_touch(0.1.0) andmob_screencast(0.1.1) are published on Hex with no rows at all, so the matrix reads as though those capabilities don't exist. Left out rather than guessed at — each needs its per-platform support checked before it earns a row. Worth a follow-up.Docs only. Note this reaches hexdocs.pm only via a published release.
🤖 Generated with Claude Code