From a376cffc2dacacdfc114bb8a84d31a22ebf33936 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sun, 30 Aug 2026 16:50:15 -0600 Subject: [PATCH 1/4] fix(android): reject stale event handles Tag event handles by render generation so callbacks cannot resolve against a replacement table. Copy event tags while their native environment remains protected. --- AGENTS.md | 8 +++ android/jni/mob_nif.zig | 90 ++++++++++++++++----------- android/jni/tap_handle_codec.zig | 64 +++++++++++++++++++ test/mob/native_event_handle_test.exs | 65 +++++++++++++++++++ 4 files changed, 189 insertions(+), 38 deletions(-) create mode 100644 android/jni/tap_handle_codec.zig create mode 100644 test/mob/native_event_handle_test.exs diff --git a/AGENTS.md b/AGENTS.md index aeaae27..9145467 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -156,6 +156,14 @@ These are the things we've burned ourselves on. Following them isn't optional. build a tree and hand it to `Mob.Sender.render/5`; the sender is the only caller. See `decisions/2026-08-28-sender-serialises-render.md`. + Android event handles also encode the render generation. `clear_taps` + advances the build generation, and `set_root` commits the table, count, and + generation under the same mutex. Every active-table lookup must decode and + validate through `resolveActiveTapLocked`; treating a handle as a bare slot + can route a callback from an old Compose tree into the current screen. A + sender must also copy the tag into its delivery environment while holding + that mutex; the table's `tag_env` may be freed as soon as the lock is released. + 4. **TDD discipline in mob_dev.** Every new public function gets a test. `mob_dev/CLAUDE.md` makes this explicit. Don't bypass — the tests are how we catch the multi-step regressions like the iOS-device deploy chain. diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index 4d24ad0..5c333a7 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -40,6 +40,7 @@ const std = @import("std"); const jni = @import("mob_zig.zig"); const erts = @import("mob_erts.zig"); +const tap_handle_codec = @import("tap_handle_codec.zig"); // ── Logging tag for NIFs that log to Android logcat ────────────────────── @@ -950,7 +951,7 @@ export fn nif_swipe_xy( // Both pools sit behind mutexes. The mutexes are created lazily by // mob_nif_init_state (called from mob_nif.c's nif_load BEAM callback). -const MAX_TAP_HANDLES: usize = 256; +const MAX_TAP_HANDLES: usize = tap_handle_codec.slot_count; // MOB-100: bumped from 64 — a single screen legitimately rendering ~60 // components (e.g. an icon catalog) plus a few leftover slots from prior // navigation could tip over the old cap. Keep in sync with the identical @@ -993,7 +994,9 @@ const ComponentHandle = extern struct { var tap_tables: [2][MAX_TAP_HANDLES]TapHandle = std.mem.zeroes([2][MAX_TAP_HANDLES]TapHandle); var tap_active: usize = 0; // index of the table readers resolve against var tap_active_count: c_int = 0; // committed handle count in the active table +var tap_active_generation: u32 = 0; // generation encoded in committed native handles var tap_build_count: c_int = 0; // handles registered so far into the building table +var tap_build_generation: u32 = 0; // generation encoded while building the next tree var tap_mutex: ?*erts.ErlNifMutex = null; /// Snapshotted by nif_set_root; written by nif_set_transition. Guarded by /// tap_mutex (the C original reused that mutex rather than allocating a @@ -1020,11 +1023,10 @@ pub export fn mob_nif_init_state() callconv(.c) c_int { } // ── Sender helpers ─────────────────────────────────────────────────────── -// All senders share the same shape: lock tap_mutex, validate the handle -// is in use (slot index in range AND tag_env non-null), copy the pid + tag -// out under the lock, then build and deliver the message to that pid in a -// freshly allocated env. The lock is dropped before enif_send so we don't -// hold it across a potentially-blocking send. +// All senders allocate their delivery env first, then lock tap_mutex, validate +// the handle, and copy the tag into that delivery env while its source tag_env +// is protected. The lock is dropped before message construction and enif_send +// so we don't hold it across potentially-blocking work. /// Snapshot a TapHandle's routing under the tap_mutex. Returns null if /// the handle is unused/out of range. The boolean flag pulls seq too — @@ -1035,24 +1037,36 @@ const TapSnap = struct { seq: u64, }; -fn snapTap(handle: c_int) ?TapSnap { +fn resolveActiveTapLocked(handle: c_int) ?*TapHandle { + const slot_index = tap_handle_codec.slotForActive( + handle, + tap_active_generation, + @intCast(tap_active_count), + ) orelse return null; + const tap = &tap_tables[tap_active][slot_index]; + return if (tap.tag_env == null) null else tap; +} + +fn snapTap(handle: c_int, env: ?*erts.ErlNifEnv) ?TapSnap { erts.enif_mutex_lock(tap_mutex); - defer erts.enif_mutex_unlock(tap_mutex); - if (handle < 0 or handle >= tap_active_count) return null; - const h = &tap_tables[tap_active][@intCast(handle)]; - if (h.tag_env == null) return null; - return TapSnap{ .pid = h.pid, .tag = h.tag, .seq = h.seq }; + const h = resolveActiveTapLocked(handle) orelse { + erts.enif_mutex_unlock(tap_mutex); + return null; + }; + const snap = TapSnap{ .pid = h.pid, .tag = erts.enif_make_copy(env, h.tag), .seq = h.seq }; + erts.enif_mutex_unlock(tap_mutex); + return snap; } /// `{:event, tag}` — used by focus/blur/submit/select and the gesture /// senders that don't carry a payload. fn sendEvent(handle: c_int, comptime atom_name: [:0]const u8) void { - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, atom_name.ptr), - erts.enif_make_copy(env, snap.tag), + snap.tag, }); var pid = snap.pid; _ = erts.enif_send(null, &pid, env, msg); @@ -1061,12 +1075,12 @@ fn sendEvent(handle: c_int, comptime atom_name: [:0]const u8) void { /// `{:change, tag, value}` — used by the three change senders below. The /// value term must originate in the same env we're delivering through. fn sendChange(handle: c_int, value_term: erts.ERL_NIF_TERM) void { - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "change"), - erts.enif_make_copy(env, snap.tag), + snap.tag, erts.enif_make_copy(env, value_term), }); var pid = snap.pid; @@ -1138,9 +1152,9 @@ pub export fn mob_send_select(handle: c_int) callconv(.c) void { /// `{:compose, tag, %{text, phase}}` — IME composition events. phase is /// began | updating | committed | cancelled (the latter two are terminal). pub export fn mob_send_compose(handle: c_int, text: ?[*:0]const u8, phase: [*:0]const u8) callconv(.c) void { - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const text_cstr: [*:0]const u8 = text orelse ""; const keys = [_]erts.ERL_NIF_TERM{ @@ -1154,7 +1168,7 @@ pub export fn mob_send_compose(handle: c_int, text: ?[*:0]const u8, phase: [*:0] const payload = erts.makeMap(env, &keys, &vals) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "compose"), - erts.enif_make_copy(env, snap.tag), + snap.tag, payload, }); var pid = snap.pid; @@ -1186,12 +1200,12 @@ pub export fn mob_send_swipe_down(handle: c_int) callconv(.c) void { } pub export fn mob_send_swipe_with_direction(handle: c_int, direction: [*:0]const u8) callconv(.c) void { - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "swipe"), - erts.enif_make_copy(env, snap.tag), + snap.tag, erts.enif_make_atom(env, direction), }); var pid = snap.pid; @@ -1214,9 +1228,7 @@ pub export fn mob_set_throttle_config( ) callconv(.c) void { erts.enif_mutex_lock(tap_mutex); defer erts.enif_mutex_unlock(tap_mutex); - if (handle < 0 or handle >= tap_active_count) return; - const h = &tap_tables[tap_active][@intCast(handle)]; - if (h.tag_env == null) return; + const h = resolveActiveTapLocked(handle) orelse return; h.throttle_ms = throttle_ms; h.debounce_ms = debounce_ms; h.delta_threshold = delta_threshold; @@ -1231,9 +1243,7 @@ pub export fn mob_set_throttle_config( fn throttleCheck(handle: c_int, x: f64, y: f64, default_throttle_ms: i32, default_delta: f64) bool { erts.enif_mutex_lock(tap_mutex); defer erts.enif_mutex_unlock(tap_mutex); - if (handle < 0 or handle >= tap_active_count) return false; - const h = &tap_tables[tap_active][@intCast(handle)]; - if (h.tag_env == null) return false; + const h = resolveActiveTapLocked(handle) orelse return false; const throttle_ms: i32 = if (h.throttle_ms != 0) h.throttle_ms else default_throttle_ms; const delta_threshold: f64 = if (h.delta_threshold > 0) h.delta_threshold else default_delta; @@ -1310,14 +1320,14 @@ pub export fn mob_send_scroll( phase: [*:0]const u8, ) callconv(.c) void { if (!isPhaseBoundary(phase) and !throttleCheck(handle, x, y, 33, 1.0)) return; - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const ts_ms = @divTrunc(jni.nowNs(), 1_000_000); const payload = buildScrollMap(env, x, y, dx, dy, vx, vy, phase, ts_ms, snap.seq); const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "scroll"), - erts.enif_make_copy(env, snap.tag), + snap.tag, payload, }); var pid = snap.pid; @@ -1333,9 +1343,9 @@ pub export fn mob_send_drag( phase: [*:0]const u8, ) callconv(.c) void { if (!isPhaseBoundary(phase) and !throttleCheck(handle, x, y, 16, 1.0)) return; - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const ts_ms = @divTrunc(jni.nowNs(), 1_000_000); const keys = [_]erts.ERL_NIF_TERM{ erts.enif_make_atom(env, "x"), @@ -1358,7 +1368,7 @@ pub export fn mob_send_drag( const payload = erts.makeMap(env, &keys, &vals) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "drag"), - erts.enif_make_copy(env, snap.tag), + snap.tag, payload, }); var pid = snap.pid; @@ -1367,9 +1377,9 @@ pub export fn mob_send_drag( pub export fn mob_send_pinch(handle: c_int, scale: f64, velocity: f64, phase: [*:0]const u8) callconv(.c) void { if (!isPhaseBoundary(phase) and !throttleCheck(handle, scale, 0, 16, 0.01)) return; - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const ts_ms = @divTrunc(jni.nowNs(), 1_000_000); const keys = [_]erts.ERL_NIF_TERM{ erts.enif_make_atom(env, "scale"), @@ -1388,7 +1398,7 @@ pub export fn mob_send_pinch(handle: c_int, scale: f64, velocity: f64, phase: [* const payload = erts.makeMap(env, &keys, &vals) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "pinch"), - erts.enif_make_copy(env, snap.tag), + snap.tag, payload, }); var pid = snap.pid; @@ -1397,9 +1407,9 @@ pub export fn mob_send_pinch(handle: c_int, scale: f64, velocity: f64, phase: [* pub export fn mob_send_rotate(handle: c_int, degrees: f64, velocity: f64, phase: [*:0]const u8) callconv(.c) void { if (!isPhaseBoundary(phase) and !throttleCheck(handle, degrees, 0, 16, 1.0)) return; - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const ts_ms = @divTrunc(jni.nowNs(), 1_000_000); const keys = [_]erts.ERL_NIF_TERM{ erts.enif_make_atom(env, "degrees"), @@ -1418,7 +1428,7 @@ pub export fn mob_send_rotate(handle: c_int, degrees: f64, velocity: f64, phase: const payload = erts.makeMap(env, &keys, &vals) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "rotate"), - erts.enif_make_copy(env, snap.tag), + snap.tag, payload, }); var pid = snap.pid; @@ -1427,9 +1437,9 @@ pub export fn mob_send_rotate(handle: c_int, degrees: f64, velocity: f64, phase: pub export fn mob_send_pointer_move(handle: c_int, x: f64, y: f64) callconv(.c) void { if (!throttleCheck(handle, x, y, 33, 4.0)) return; - const snap = snapTap(handle) orelse return; const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); + const snap = snapTap(handle, env) orelse return; const ts_ms = @divTrunc(jni.nowNs(), 1_000_000); const keys = [_]erts.ERL_NIF_TERM{ erts.enif_make_atom(env, "x"), @@ -1446,7 +1456,7 @@ pub export fn mob_send_pointer_move(handle: c_int, x: f64, y: f64) callconv(.c) const payload = erts.makeMap(env, &keys, &vals) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "pointer_move"), - erts.enif_make_copy(env, snap.tag), + snap.tag, payload, }); var pid = snap.pid; @@ -1564,6 +1574,7 @@ export fn nif_set_root( // any send racing this swap sees a complete table on either side. tap_active = 1 - tap_active; tap_active_count = tap_build_count; + tap_active_generation = tap_build_generation; erts.enif_mutex_unlock(tap_mutex); const transition_cstr: [*:0]const u8 = @ptrCast(&transition); @@ -1617,9 +1628,11 @@ export fn nif_register_tap( return erts.enif_make_int(env, -1); } - const handle: c_int = tap_build_count; + const slot_index: usize = @intCast(tap_build_count); + const handle = tap_handle_codec.encode(tap_build_generation, slot_index) orelse + return erts.enif_make_int(env, -1); tap_build_count += 1; - const slot = &tap_tables[1 - tap_active][@intCast(handle)]; + const slot = &tap_tables[1 - tap_active][slot_index]; slot.pid = pid; slot.tag_env = erts.enif_alloc_env() orelse return erts.atom(env, "error"); slot.tag = erts.enif_make_copy(slot.tag_env, tag_term); @@ -1638,6 +1651,7 @@ export fn nif_clear_taps( _ = argv; erts.enif_mutex_lock(tap_mutex); defer erts.enif_mutex_unlock(tap_mutex); + tap_build_generation = tap_handle_codec.nextGeneration(tap_build_generation); // Prepare the INACTIVE (building) table for a fresh frame; leave the active // table intact so concurrent mob_send_* keep resolving the last committed // frame. The freshly built table is swapped in at set_root. diff --git a/android/jni/tap_handle_codec.zig b/android/jni/tap_handle_codec.zig new file mode 100644 index 0000000..85662b5 --- /dev/null +++ b/android/jni/tap_handle_codec.zig @@ -0,0 +1,64 @@ +const std = @import("std"); + +pub const slot_count: usize = 256; +pub const max_generation: u32 = 0x7fffff; + +pub const Decoded = struct { + generation: u32, + slot: usize, +}; + +pub fn encode(generation: u32, slot: usize) ?i32 { + if (generation == 0 or generation > max_generation or slot >= slot_count) return null; + const raw = (generation << 8) | @as(u32, @intCast(slot)); + return @intCast(raw); +} + +pub fn decode(handle: i32) ?Decoded { + if (handle <= 0) return null; + const raw: u32 = @intCast(handle); + const generation = raw >> 8; + if (generation == 0) return null; + return .{ .generation = generation, .slot = raw & 0xff }; +} + +pub fn slotForActive(handle: i32, active_generation: u32, active_count: usize) ?usize { + const decoded = decode(handle) orelse return null; + if (decoded.generation != active_generation or decoded.slot >= active_count) return null; + return decoded.slot; +} + +pub fn nextGeneration(generation: u32) u32 { + return if (generation == 0 or generation >= max_generation) 1 else generation + 1; +} + +test "round trips every slot through a positive generation-tagged handle" { + for (0..slot_count) |slot| { + const handle = encode(42, slot).?; + try std.testing.expect(handle > 0); + try std.testing.expectEqual(Decoded{ .generation = 42, .slot = slot }, decode(handle).?); + } +} + +test "rejects invalid handles and encoding inputs" { + try std.testing.expect(decode(-1) == null); + try std.testing.expect(decode(0) == null); + try std.testing.expect(encode(0, 0) == null); + try std.testing.expect(encode(1, slot_count) == null); + try std.testing.expect(encode(max_generation + 1, 0) == null); +} + +test "validates generation and committed slot count together" { + const handle = encode(9, 37).?; + + try std.testing.expectEqual(@as(?usize, 37), slotForActive(handle, 9, 38)); + try std.testing.expect(slotForActive(handle, 8, 38) == null); + try std.testing.expect(slotForActive(handle, 9, 37) == null); + try std.testing.expect(slotForActive(-1, 9, 38) == null); +} + +test "generation wraps to one instead of producing invalid handles" { + try std.testing.expectEqual(@as(u32, 2), nextGeneration(1)); + try std.testing.expectEqual(@as(u32, 1), nextGeneration(max_generation)); + try std.testing.expectEqual(@as(u32, 1), nextGeneration(0)); +} diff --git a/test/mob/native_event_handle_test.exs b/test/mob/native_event_handle_test.exs new file mode 100644 index 0000000..6a7e75d --- /dev/null +++ b/test/mob/native_event_handle_test.exs @@ -0,0 +1,65 @@ +# Android event handles cross the Compose/JNI boundary as integers, so host +# tests pin the native source contract while the pure Zig codec covers values. +# credo:disable-for-this-file Jump.CredoChecks.VacuousTest +defmodule Mob.NativeEventHandleTest do + use ExUnit.Case, async: true + + @source_path Path.expand("../../android/jni/mob_nif.zig", __DIR__) + @source File.read!(@source_path) + + test "Android event handles carry the render generation" do + assert @source =~ ~s|const tap_handle_codec = @import("tap_handle_codec.zig")| + assert @source =~ "var tap_active_generation: u32 = 0" + assert @source =~ "var tap_build_generation: u32 = 0" + + assert @source =~ + "tap_build_generation = tap_handle_codec.nextGeneration(tap_build_generation)" + + assert @source =~ "tap_handle_codec.encode(tap_build_generation, slot_index)" + end + + test "active table, count, and generation commit under one lock" do + [_, commit] = String.split(@source, "// Commit the freshly-built tap table:", parts: 2) + [commit, _] = String.split(commit, "erts.enif_mutex_unlock(tap_mutex);", parts: 2) + + assert commit =~ "tap_active = 1 - tap_active" + assert commit =~ "tap_active_count = tap_build_count" + assert commit =~ "tap_active_generation = tap_build_generation" + end + + test "all active event-table lookups share generation validation" do + assert @source =~ "fn resolveActiveTapLocked(handle: c_int) ?*TapHandle" + + assert length(Regex.scan(~r/resolveActiveTapLocked\(handle\)/, @source)) == 3, + "snapTap, mob_set_throttle_config, and throttleCheck must use the shared lookup" + + refute @source =~ "handle >= tap_active_count" + refute @source =~ "tap_tables[tap_active][@intCast(handle)]" + end + + test "sender tags are copied before the tap-table lock is released" do + [_, snap] = String.split(@source, "fn snapTap", parts: 2) + [snap, _] = String.split(snap, "/// `{:event, tag}`", parts: 2) + + {lock, _} = :binary.match(snap, "erts.enif_mutex_lock(tap_mutex)") + {copy, _} = :binary.match(snap, "erts.enif_make_copy(env, h.tag)") + {unlock, _} = :binary.matches(snap, "erts.enif_mutex_unlock(tap_mutex)") |> List.last() + + assert lock < copy and copy < unlock + assert snap =~ "resolveActiveTapLocked(handle) orelse {" + assert length(:binary.matches(snap, "erts.enif_mutex_unlock(tap_mutex)")) == 2 + end + + test "every tag snapshot owns and frees its delivery environment" do + snapshots = Regex.scan(~r/snapTap\(handle, env\)/, @source) + + allocated_snapshots = + Regex.scan( + ~r/const env = erts\.enif_alloc_env\(\) orelse return;\s+defer erts\.enif_free_env\(env\);\s+const snap = snapTap\(handle, env\) orelse return;/, + @source + ) + + assert length(snapshots) == 9 + assert length(allocated_snapshots) == length(snapshots) + end +end From 083451ba1919db7cd001d9e8a4586a5575821839 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sun, 30 Aug 2026 18:18:24 -0600 Subject: [PATCH 2/4] fix(native): preserve event identity Accept in-flight change events only when their prior and current registrations match. Extend generation checks to iOS and persistent components. --- AGENTS.md | 15 +- CHANGELOG.md | 14 + android/jni/mob_erts.zig | 1 + android/jni/mob_nif.zig | 107 ++++++-- ios/mob_nif.m | 377 ++++++++++++++++---------- test/mob/native_event_handle_test.exs | 88 ++++-- 6 files changed, 416 insertions(+), 186 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9145467..9a4a2ea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -156,13 +156,14 @@ These are the things we've burned ourselves on. Following them isn't optional. build a tree and hand it to `Mob.Sender.render/5`; the sender is the only caller. See `decisions/2026-08-28-sender-serialises-render.md`. - Android event handles also encode the render generation. `clear_taps` - advances the build generation, and `set_root` commits the table, count, and - generation under the same mutex. Every active-table lookup must decode and - validate through `resolveActiveTapLocked`; treating a handle as a bare slot - can route a callback from an old Compose tree into the current screen. A - sender must also copy the tag into its delivery environment while holding - that mutex; the table's `tag_env` may be freed as soon as the lock is released. + Native event handles encode the render generation on both platforms. + `clear_taps` advances the build generation, and `set_root` commits the table, + count, and generation under the same mutex. Treating a handle as a bare slot + can route a callback from an old native tree into the current screen. A sender + must also copy the tag into its delivery environment while holding that + mutex; the table's `tag_env` may be freed as soon as the lock is released. + Change events alone may cross one render when both retained registrations + have identical PID and tag identity; taps and gestures stay generation-strict. 4. **TDD discipline in mob_dev.** Every new public function gets a test. `mob_dev/CLAUDE.md` makes this explicit. Don't bypass — the tests are how we diff --git a/CHANGELOG.md b/CHANGELOG.md index 973ed51..05106f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,20 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). ## [Unreleased] +### Fixed +- **Stale native callbacks no longer route to replacement handlers.** Android + and iOS event handles now carry their render generation, with the handler + table, count, and generation committed atomically. Taps and gestures from an + old native tree are rejected; text, toggle, and slider changes may cross one + render only when the retained and current registrations have identical PID + and tag identity, preserving in-flight input without permitting misroutes. + Event tags are copied while their registry lock is held, closing the iOS and + Android environment-lifetime race. Persistent component handles also carry a + per-slot generation so callbacks from reclaimed slots cannot reach a new + component. Rejections are visible in debug native logs. Generated Android + projects must pair this with the companion generator update that keys list + state independently of the full event handle. + ## [0.7.37] - 2026-08-30 ### Added diff --git a/android/jni/mob_erts.zig b/android/jni/mob_erts.zig index 5dae795..9761f37 100644 --- a/android/jni/mob_erts.zig +++ b/android/jni/mob_erts.zig @@ -194,6 +194,7 @@ pub inline fn enif_make_uint64(env: ?*ErlNifEnv, i: u64) ERL_NIF_TERM { pub extern fn enif_alloc_env() ?*ErlNifEnv; pub extern fn enif_free_env(env: ?*ErlNifEnv) void; pub extern fn enif_make_copy(dst: ?*ErlNifEnv, src_term: ERL_NIF_TERM) ERL_NIF_TERM; +pub extern fn enif_compare(lhs: ERL_NIF_TERM, rhs: ERL_NIF_TERM) c_int; pub extern fn enif_send( caller_env: ?*ErlNifEnv, to_pid: *const ErlNifPid, diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index 5c333a7..35fe217 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -994,7 +994,7 @@ const ComponentHandle = extern struct { var tap_tables: [2][MAX_TAP_HANDLES]TapHandle = std.mem.zeroes([2][MAX_TAP_HANDLES]TapHandle); var tap_active: usize = 0; // index of the table readers resolve against var tap_active_count: c_int = 0; // committed handle count in the active table -var tap_active_generation: u32 = 0; // generation encoded in committed native handles +var tap_table_generations: [2]u32 = .{ 0, 0 }; var tap_build_count: c_int = 0; // handles registered so far into the building table var tap_build_generation: u32 = 0; // generation encoded while building the next tree var tap_mutex: ?*erts.ErlNifMutex = null; @@ -1011,6 +1011,7 @@ var g_transition: [16]u8 = blk: { }; var component_handles: [MAX_COMPONENT_HANDLES]ComponentHandle = @splat(std.mem.zeroes(ComponentHandle)); +var component_generations: [MAX_COMPONENT_HANDLES]u32 = @splat(0); var component_mutex: ?*erts.ErlNifMutex = null; /// Initialise both mutexes. Called from mob_nif.c's nif_load BEAM callback @@ -1040,20 +1041,73 @@ const TapSnap = struct { fn resolveActiveTapLocked(handle: c_int) ?*TapHandle { const slot_index = tap_handle_codec.slotForActive( handle, - tap_active_generation, + tap_table_generations[tap_active], @intCast(tap_active_count), ) orelse return null; const tap = &tap_tables[tap_active][slot_index]; return if (tap.tag_env == null) null else tap; } +fn resolveGenerationTapLocked(handle: c_int) ?*TapHandle { + const decoded = tap_handle_codec.decode(handle) orelse return null; + for (0..tap_tables.len) |table_index| { + if (tap_table_generations[table_index] != decoded.generation) continue; + const tap = &tap_tables[table_index][decoded.slot]; + return if (tap.tag_env == null) null else tap; + } + return null; +} + +fn copyTap(tap: *const TapHandle, env: ?*erts.ErlNifEnv) TapSnap { + return .{ .pid = tap.pid, .tag = erts.enif_make_copy(env, tap.tag), .seq = tap.seq }; +} + fn snapTap(handle: c_int, env: ?*erts.ErlNifEnv) ?TapSnap { erts.enif_mutex_lock(tap_mutex); const h = resolveActiveTapLocked(handle) orelse { erts.enif_mutex_unlock(tap_mutex); + logd_nif("rejected stale event handle {d}", .{handle}); return null; }; - const snap = TapSnap{ .pid = h.pid, .tag = erts.enif_make_copy(env, h.tag), .seq = h.seq }; + const snap = copyTap(h, env); + erts.enif_mutex_unlock(tap_mutex); + return snap; +} + +fn snapChangeTap(handle: c_int, env: ?*erts.ErlNifEnv) ?TapSnap { + erts.enif_mutex_lock(tap_mutex); + if (resolveActiveTapLocked(handle)) |active| { + const snap = copyTap(active, env); + erts.enif_mutex_unlock(tap_mutex); + return snap; + } + + const decoded = tap_handle_codec.decode(handle) orelse { + erts.enif_mutex_unlock(tap_mutex); + logd_nif("rejected stale event handle {d}", .{handle}); + return null; + }; + const source = resolveGenerationTapLocked(handle) orelse { + erts.enif_mutex_unlock(tap_mutex); + logd_nif("rejected stale event handle {d}", .{handle}); + return null; + }; + const active = if (decoded.slot < @as(usize, @intCast(tap_active_count))) + &tap_tables[tap_active][decoded.slot] + else { + erts.enif_mutex_unlock(tap_mutex); + logd_nif("rejected stale event handle {d}", .{handle}); + return null; + }; + if (active.tag_env == null or source.pid.pid != active.pid.pid or + erts.enif_compare(source.tag, active.tag) != 0) + { + erts.enif_mutex_unlock(tap_mutex); + logd_nif("rejected stale event handle {d}", .{handle}); + return null; + } + + const snap = copyTap(active, env); erts.enif_mutex_unlock(tap_mutex); return snap; } @@ -1077,7 +1131,7 @@ fn sendEvent(handle: c_int, comptime atom_name: [:0]const u8) void { fn sendChange(handle: c_int, value_term: erts.ERL_NIF_TERM) void { const env = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(env); - const snap = snapTap(handle, env) orelse return; + const snap = snapChangeTap(handle, env) orelse return; const msg = erts.makeTuple(env, .{ erts.enif_make_atom(env, "change"), snap.tag, @@ -1483,18 +1537,28 @@ pub export fn mob_send_scrolled_past(handle: c_int) callconv(.c) void { // ── Component event sender ────────────────────────────────────────────── +fn decodeComponentHandle(handle: c_int) ?usize { + const decoded = tap_handle_codec.decode(handle) orelse return null; + if (decoded.slot >= MAX_COMPONENT_HANDLES or + component_generations[decoded.slot] != decoded.generation or + component_handles[decoded.slot].active == 0) + { + return null; + } + return decoded.slot; +} + pub export fn mob_send_component_event( handle: c_int, event: [*:0]const u8, payload_json: [*:0]const u8, ) callconv(.c) void { - if (handle < 0 or handle >= @as(c_int, @intCast(MAX_COMPONENT_HANDLES))) return; erts.enif_mutex_lock(component_mutex); - const slot = &component_handles[@intCast(handle)]; - if (slot.active == 0) { + const slot_index = decodeComponentHandle(handle) orelse { erts.enif_mutex_unlock(component_mutex); return; - } + }; + const slot = &component_handles[slot_index]; const pid_copy = slot.pid; erts.enif_mutex_unlock(component_mutex); @@ -1574,7 +1638,7 @@ export fn nif_set_root( // any send racing this swap sees a complete table on either side. tap_active = 1 - tap_active; tap_active_count = tap_build_count; - tap_active_generation = tap_build_generation; + tap_table_generations[tap_active] = tap_build_generation; erts.enif_mutex_unlock(tap_mutex); const transition_cstr: [*:0]const u8 = @ptrCast(&transition); @@ -1629,8 +1693,10 @@ export fn nif_register_tap( } const slot_index: usize = @intCast(tap_build_count); - const handle = tap_handle_codec.encode(tap_build_generation, slot_index) orelse + const handle = tap_handle_codec.encode(tap_build_generation, slot_index) orelse { + loge_nif("register_tap: invalid generation {d}", .{tap_build_generation}); return erts.enif_make_int(env, -1); + }; tap_build_count += 1; const slot = &tap_tables[1 - tap_active][slot_index]; slot.pid = pid; @@ -1718,9 +1784,11 @@ export fn nif_register_component( var i: usize = 0; while (i < MAX_COMPONENT_HANDLES) : (i += 1) { if (component_handles[i].active == 0) { + component_generations[i] = tap_handle_codec.nextGeneration(component_generations[i]); + const handle = tap_handle_codec.encode(component_generations[i], i) orelse unreachable; component_handles[i].pid = pid; component_handles[i].active = 1; - return erts.makeTuple(env, .{ erts.atom(env, "ok"), erts.enif_make_int(env, @intCast(i)) }); + return erts.makeTuple(env, .{ erts.atom(env, "ok"), erts.enif_make_int(env, handle) }); } } return erts.errorTuple(env, erts.atom(env, "component_slots_exhausted")); @@ -1735,14 +1803,13 @@ export fn nif_deregister_component( ) callconv(.c) erts.ERL_NIF_TERM { _ = argc; var handle: c_int = 0; - if (erts.enif_get_int(env, argv[0], &handle) == 0 or - handle < 0 or - handle >= @as(c_int, @intCast(MAX_COMPONENT_HANDLES))) - { - return erts.badarg(env); - } + if (erts.enif_get_int(env, argv[0], &handle) == 0) return erts.badarg(env); erts.enif_mutex_lock(component_mutex); - component_handles[@intCast(handle)].active = 0; + const slot_index = decodeComponentHandle(handle) orelse { + erts.enif_mutex_unlock(component_mutex); + return erts.badarg(env); + }; + component_handles[slot_index].active = 0; erts.enif_mutex_unlock(component_mutex); return erts.ok(env); } @@ -1760,6 +1827,10 @@ inline fn logi_nif(comptime fmt: []const u8, args: anytype) void { jni.logWrite(jni.ANDROID_LOG_INFO, NIF_LOG_TAG, fmt, args); } +inline fn logd_nif(comptime fmt: []const u8, args: anytype) void { + jni.logWrite(jni.ANDROID_LOG_DEBUG, NIF_LOG_TAG, fmt, args); +} + inline fn loge_nif(comptime fmt: []const u8, args: anytype) void { jni.logWrite(jni.ANDROID_LOG_ERROR, NIF_LOG_TAG, fmt, args); } diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 5a40f02..0101828 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -47,6 +47,11 @@ #define LOGI(...) NSLog(@"[MobNIF] " __VA_ARGS__) #define LOGE(...) NSLog(@"[MobNIF][ERROR] " __VA_ARGS__) +#if DEBUG +#define LOGD(...) NSLog(@"[MobNIF][DEBUG] " __VA_ARGS__) +#else +#define LOGD(...) +#endif // ── Startup status (declared in mob_beam.h, called from mob_beam.m) ─────────── // Implemented here rather than in mob_beam.m because this file is compiled with @@ -66,6 +71,7 @@ void mob_set_startup_error(const char *error) { // Cleared before every render. Max 256 tappable elements per frame. #define MAX_TAP_HANDLES 256 +#define MAX_EVENT_GENERATION 0x7fffffU typedef struct { ErlNifPid pid; @@ -95,8 +101,95 @@ void mob_set_startup_error(const char *error) { static TapHandle *tap_handles = tap_tables[0]; // active table (readers use this) static int tap_handle_next = 0; // active committed count (readers' bound) static int tap_build_count = 0; // cursor into the building table +static uint32_t tap_table_generations[2] = {0, 0}; +static uint32_t tap_build_generation = 0; static ErlNifMutex *tap_mutex = NULL; +static uint32_t mob_next_handle_generation(uint32_t generation) { + return generation == 0 || generation >= MAX_EVENT_GENERATION ? 1 : generation + 1; +} + +static int mob_encode_event_handle(uint32_t generation, int slot) { + if (generation == 0 || generation > MAX_EVENT_GENERATION || slot < 0 || slot >= MAX_TAP_HANDLES) + return -1; + return (int)((generation << 8) | (uint32_t)slot); +} + +static int mob_decode_event_handle(int handle, uint32_t *generation, int *slot) { + if (handle <= 0) + return 0; + uint32_t raw = (uint32_t)handle; + *generation = raw >> 8; + *slot = (int)(raw & 0xffU); + return *generation != 0; +} + +typedef struct { + ErlNifPid pid; + ERL_NIF_TERM tag; + uint64_t seq; +} TapSnap; + +static TapHandle *mob_resolve_active_tap_locked(int handle) { + uint32_t generation; + int slot; + if (!mob_decode_event_handle(handle, &generation, &slot) || + generation != tap_table_generations[tap_active] || slot >= tap_handle_next || + !tap_handles[slot].tag_env) + return NULL; + return &tap_handles[slot]; +} + +static TapHandle *mob_resolve_generation_tap_locked(int handle, int *slot_out) { + uint32_t generation; + int slot; + if (!mob_decode_event_handle(handle, &generation, &slot)) + return NULL; + for (int table = 0; table < 2; table++) { + if (tap_table_generations[table] == generation && tap_tables[table][slot].tag_env) { + *slot_out = slot; + return &tap_tables[table][slot]; + } + } + return NULL; +} + +static int mob_snap_tap(int handle, ErlNifEnv *msg_env, TapSnap *snap) { + enif_mutex_lock(tap_mutex); + TapHandle *active = mob_resolve_active_tap_locked(handle); + if (!active) { + enif_mutex_unlock(tap_mutex); + LOGD(@"rejected stale event handle %d", handle); + return 0; + } + snap->pid = active->pid; + snap->tag = enif_make_copy(msg_env, active->tag); + snap->seq = active->seq; + enif_mutex_unlock(tap_mutex); + return 1; +} + +static int mob_snap_change_tap(int handle, ErlNifEnv *msg_env, TapSnap *snap) { + enif_mutex_lock(tap_mutex); + TapHandle *active = mob_resolve_active_tap_locked(handle); + if (!active) { + int slot = -1; + TapHandle *source = mob_resolve_generation_tap_locked(handle, &slot); + active = slot >= 0 && slot < tap_handle_next ? &tap_handles[slot] : NULL; + if (!source || !active || !active->tag_env || source->pid.pid != active->pid.pid || + enif_compare(source->tag, active->tag) != 0) { + enif_mutex_unlock(tap_mutex); + LOGD(@"rejected stale event handle %d", handle); + return 0; + } + } + snap->pid = active->pid; + snap->tag = enif_make_copy(msg_env, active->tag); + snap->seq = active->seq; + enif_mutex_unlock(tap_mutex); + return 1; +} + // Convert mach absolute time to nanoseconds (initialised once). static mach_timebase_info_data_t g_timebase = {0, 0}; static uint64_t mob_now_ns(void) { @@ -110,12 +203,13 @@ static uint64_t mob_now_ns(void) { static void mob_set_throttle_config(int handle, int throttle_ms, int debounce_ms, double delta_threshold, int leading, int trailing) { enif_mutex_lock(tap_mutex); - if (handle >= 0 && handle < tap_handle_next && tap_handles[handle].tag_env) { - tap_handles[handle].throttle_ms = throttle_ms; - tap_handles[handle].debounce_ms = debounce_ms; - tap_handles[handle].delta_threshold = delta_threshold; - tap_handles[handle].leading = leading; - tap_handles[handle].trailing = trailing; + TapHandle *tap = mob_resolve_active_tap_locked(handle); + if (tap) { + tap->throttle_ms = throttle_ms; + tap->debounce_ms = debounce_ms; + tap->delta_threshold = delta_threshold; + tap->leading = leading; + tap->trailing = trailing; } enif_mutex_unlock(tap_mutex); } @@ -129,11 +223,11 @@ static void mob_set_throttle_config(int handle, int throttle_ms, int debounce_ms static int mob_throttle_check(int handle, double x, double y, int default_throttle_ms, double default_delta) { enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { + TapHandle *h = mob_resolve_active_tap_locked(handle); + if (!h) { enif_mutex_unlock(tap_mutex); return 0; } - TapHandle *h = &tap_handles[handle]; int throttle_ms = h->throttle_ms ? h->throttle_ms : default_throttle_ms; double delta_threshold = h->delta_threshold > 0 ? h->delta_threshold : default_delta; @@ -169,8 +263,9 @@ static int mob_throttle_check(int handle, double x, double y, int default_thrott // Read current seq + ts for a handle (for envelope construction). static void mob_handle_meta(int handle, uint64_t *seq_out, uint64_t *ts_out) { enif_mutex_lock(tap_mutex); - if (handle >= 0 && handle < tap_handle_next && tap_handles[handle].tag_env) { - *seq_out = tap_handles[handle].seq; + TapHandle *tap = mob_resolve_active_tap_locked(handle); + if (tap) { + *seq_out = tap->seq; *ts_out = mob_now_ns() / 1000000ULL; // ms since boot } else { *seq_out = 0; @@ -205,20 +300,18 @@ static void mob_note_ui_event(void) { // Called from node onTap blocks — routes tap to BEAM via enif_send. static void mob_send_tap(int handle) { - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - enif_mutex_unlock(tap_mutex); mob_note_ui_event(); - ErlNifEnv *msg_env = enif_alloc_env(); - ERL_NIF_TERM msg = - enif_make_tuple2(msg_env, enif_make_atom(msg_env, "tap"), enif_make_copy(msg_env, tag)); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = enif_make_tuple2(msg_env, enif_make_atom(msg_env, "tap"), snap.tag); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -226,20 +319,18 @@ static void mob_send_tap(int handle) { // Called from MobTextField SwiftUI view when focus state changes or return key tapped. static void mob_send_event(int handle, const char *atom) { - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - enif_mutex_unlock(tap_mutex); mob_note_ui_event(); - ErlNifEnv *msg_env = enif_alloc_env(); - ERL_NIF_TERM msg = - enif_make_tuple2(msg_env, enif_make_atom(msg_env, atom), enif_make_copy(msg_env, tag)); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = enif_make_tuple2(msg_env, enif_make_atom(msg_env, atom), snap.tag); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -263,16 +354,15 @@ static void mob_send_dismiss(int handle) { // phase is one of began/updating/committed/cancelled. Called from the // text-input layer when marked-text state changes. static void mob_send_compose(int handle, const char *text, const char *phase) { - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - enif_mutex_unlock(tap_mutex); - ErlNifEnv *msg_env = enif_alloc_env(); ERL_NIF_TERM keys[2] = { enif_make_atom(msg_env, "text"), enif_make_atom(msg_env, "phase"), @@ -283,9 +373,9 @@ static void mob_send_compose(int handle, const char *text, const char *phase) { }; ERL_NIF_TERM payload; enif_make_map_from_arrays(msg_env, keys, vals, 2, &payload); - ERL_NIF_TERM msg = enif_make_tuple3(msg_env, enif_make_atom(msg_env, "compose"), - enif_make_copy(msg_env, tag), payload); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = + enif_make_tuple3(msg_env, enif_make_atom(msg_env, "compose"), snap.tag, payload); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -314,20 +404,18 @@ static void mob_send_swipe_down(int handle) { // Generic on_swipe with direction: emits {swipe, tag, direction} where direction is an atom. static void mob_send_swipe_with_direction(int handle, const char *direction) { - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - enif_mutex_unlock(tap_mutex); - ErlNifEnv *msg_env = enif_alloc_env(); - ERL_NIF_TERM msg = - enif_make_tuple3(msg_env, enif_make_atom(msg_env, "swipe"), enif_make_copy(msg_env, tag), - enif_make_atom(msg_env, direction)); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = enif_make_tuple3(msg_env, enif_make_atom(msg_env, "swipe"), snap.tag, + enif_make_atom(msg_env, direction)); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -376,22 +464,21 @@ static void mob_send_scroll(int handle, double x, double y, double dx, double dy if (!is_phase_boundary && !mob_throttle_check(handle, x, y, 33, 1.0)) return; - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - uint64_t seq = tap_handles[handle].seq; - enif_mutex_unlock(tap_mutex); uint64_t ts = mob_now_ns() / 1000000ULL; - ErlNifEnv *msg_env = enif_alloc_env(); - ERL_NIF_TERM payload = mob_build_scroll_payload(msg_env, x, y, dx, dy, vx, vy, phase, ts, seq); - ERL_NIF_TERM msg = enif_make_tuple3(msg_env, enif_make_atom(msg_env, "scroll"), - enif_make_copy(msg_env, tag), payload); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM payload = + mob_build_scroll_payload(msg_env, x, y, dx, dy, vx, vy, phase, ts, snap.seq); + ERL_NIF_TERM msg = + enif_make_tuple3(msg_env, enif_make_atom(msg_env, "scroll"), snap.tag, payload); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -400,18 +487,16 @@ static void mob_send_drag(int handle, double x, double y, double dx, double dy, if (!is_phase_boundary && !mob_throttle_check(handle, x, y, 16, 1.0)) return; - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - uint64_t seq = tap_handles[handle].seq; - enif_mutex_unlock(tap_mutex); uint64_t ts = mob_now_ns() / 1000000ULL; - ErlNifEnv *msg_env = enif_alloc_env(); // Drag payload: %{x, y, dx, dy, phase, ts, seq} ERL_NIF_TERM keys[7] = { enif_make_atom(msg_env, "x"), enif_make_atom(msg_env, "y"), @@ -420,16 +505,16 @@ static void mob_send_drag(int handle, double x, double y, double dx, double dy, enif_make_atom(msg_env, "seq"), }; ERL_NIF_TERM vals[7] = { - enif_make_double(msg_env, x), enif_make_double(msg_env, y), - enif_make_double(msg_env, dx), enif_make_double(msg_env, dy), - enif_make_atom(msg_env, phase), enif_make_uint64(msg_env, ts), - enif_make_uint64(msg_env, seq), + enif_make_double(msg_env, x), enif_make_double(msg_env, y), + enif_make_double(msg_env, dx), enif_make_double(msg_env, dy), + enif_make_atom(msg_env, phase), enif_make_uint64(msg_env, ts), + enif_make_uint64(msg_env, snap.seq), }; ERL_NIF_TERM payload; enif_make_map_from_arrays(msg_env, keys, vals, 7, &payload); - ERL_NIF_TERM msg = enif_make_tuple3(msg_env, enif_make_atom(msg_env, "drag"), - enif_make_copy(msg_env, tag), payload); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = + enif_make_tuple3(msg_env, enif_make_atom(msg_env, "drag"), snap.tag, payload); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -438,33 +523,31 @@ static void mob_send_pinch(int handle, double scale, double velocity, const char if (!is_phase_boundary && !mob_throttle_check(handle, scale, 0, 16, 0.01)) return; - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - uint64_t seq = tap_handles[handle].seq; - enif_mutex_unlock(tap_mutex); uint64_t ts = mob_now_ns() / 1000000ULL; - ErlNifEnv *msg_env = enif_alloc_env(); ERL_NIF_TERM keys[5] = { enif_make_atom(msg_env, "scale"), enif_make_atom(msg_env, "velocity"), enif_make_atom(msg_env, "phase"), enif_make_atom(msg_env, "ts"), enif_make_atom(msg_env, "seq"), }; ERL_NIF_TERM vals[5] = { - enif_make_double(msg_env, scale), enif_make_double(msg_env, velocity), - enif_make_atom(msg_env, phase), enif_make_uint64(msg_env, ts), - enif_make_uint64(msg_env, seq), + enif_make_double(msg_env, scale), enif_make_double(msg_env, velocity), + enif_make_atom(msg_env, phase), enif_make_uint64(msg_env, ts), + enif_make_uint64(msg_env, snap.seq), }; ERL_NIF_TERM payload; enif_make_map_from_arrays(msg_env, keys, vals, 5, &payload); - ERL_NIF_TERM msg = enif_make_tuple3(msg_env, enif_make_atom(msg_env, "pinch"), - enif_make_copy(msg_env, tag), payload); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = + enif_make_tuple3(msg_env, enif_make_atom(msg_env, "pinch"), snap.tag, payload); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -473,33 +556,31 @@ static void mob_send_rotate(int handle, double degrees, double velocity, const c if (!is_phase_boundary && !mob_throttle_check(handle, degrees, 0, 16, 1.0)) return; - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - uint64_t seq = tap_handles[handle].seq; - enif_mutex_unlock(tap_mutex); uint64_t ts = mob_now_ns() / 1000000ULL; - ErlNifEnv *msg_env = enif_alloc_env(); ERL_NIF_TERM keys[5] = { enif_make_atom(msg_env, "degrees"), enif_make_atom(msg_env, "velocity"), enif_make_atom(msg_env, "phase"), enif_make_atom(msg_env, "ts"), enif_make_atom(msg_env, "seq"), }; ERL_NIF_TERM vals[5] = { - enif_make_double(msg_env, degrees), enif_make_double(msg_env, velocity), - enif_make_atom(msg_env, phase), enif_make_uint64(msg_env, ts), - enif_make_uint64(msg_env, seq), + enif_make_double(msg_env, degrees), enif_make_double(msg_env, velocity), + enif_make_atom(msg_env, phase), enif_make_uint64(msg_env, ts), + enif_make_uint64(msg_env, snap.seq), }; ERL_NIF_TERM payload; enif_make_map_from_arrays(msg_env, keys, vals, 5, &payload); - ERL_NIF_TERM msg = enif_make_tuple3(msg_env, enif_make_atom(msg_env, "rotate"), - enif_make_copy(msg_env, tag), payload); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = + enif_make_tuple3(msg_env, enif_make_atom(msg_env, "rotate"), snap.tag, payload); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -507,18 +588,16 @@ static void mob_send_pointer_move(int handle, double x, double y) { if (!mob_throttle_check(handle, x, y, 33, 4.0)) return; - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - uint64_t seq = tap_handles[handle].seq; - enif_mutex_unlock(tap_mutex); uint64_t ts = mob_now_ns() / 1000000ULL; - ErlNifEnv *msg_env = enif_alloc_env(); ERL_NIF_TERM keys[4] = { enif_make_atom(msg_env, "x"), enif_make_atom(msg_env, "y"), @@ -529,13 +608,13 @@ static void mob_send_pointer_move(int handle, double x, double y) { enif_make_double(msg_env, x), enif_make_double(msg_env, y), enif_make_uint64(msg_env, ts), - enif_make_uint64(msg_env, seq), + enif_make_uint64(msg_env, snap.seq), }; ERL_NIF_TERM payload; enif_make_map_from_arrays(msg_env, keys, vals, 4, &payload); - ERL_NIF_TERM msg = enif_make_tuple3(msg_env, enif_make_atom(msg_env, "pointer_move"), - enif_make_copy(msg_env, tag), payload); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = + enif_make_tuple3(msg_env, enif_make_atom(msg_env, "pointer_move"), snap.tag, payload); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -576,21 +655,19 @@ void mob_handle_back(void) { // Called from MobNode onChange blocks when an input widget fires. static void mob_send_change(int handle, ERL_NIF_TERM value_term) { - enif_mutex_lock(tap_mutex); - if (handle < 0 || handle >= tap_handle_next || !tap_handles[handle].tag_env) { - enif_mutex_unlock(tap_mutex); + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_change_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); return; } - ErlNifPid pid = tap_handles[handle].pid; - ERL_NIF_TERM tag = tap_handles[handle].tag; - enif_mutex_unlock(tap_mutex); mob_note_ui_event(); - ErlNifEnv *msg_env = enif_alloc_env(); - ERL_NIF_TERM msg = - enif_make_tuple3(msg_env, enif_make_atom(msg_env, "change"), enif_make_copy(msg_env, tag), - enif_make_copy(msg_env, value_term)); - enif_send(NULL, &pid, msg_env, msg); + ERL_NIF_TERM msg = enif_make_tuple3(msg_env, enif_make_atom(msg_env, "change"), snap.tag, + enif_make_copy(msg_env, value_term)); + enif_send(NULL, &snap.pid, msg_env, msg); enif_free_env(msg_env); } @@ -2114,6 +2191,7 @@ static ERL_NIF_TERM nif_set_root(ErlNifEnv *env, int argc, const ERL_NIF_TERM ar tap_active = 1 - tap_active; tap_handles = tap_tables[tap_active]; tap_handle_next = tap_build_count; + tap_table_generations[tap_active] = tap_build_generation; enif_mutex_unlock(tap_mutex); // A non-"none" transition is what makes MobViewModel bump navVersion, and @@ -2169,10 +2247,17 @@ static ERL_NIF_TERM nif_register_tap(ErlNifEnv *env, int argc, const ERL_NIF_TER return enif_make_int(env, -1); } TapHandle *build = tap_tables[1 - tap_active]; - int handle = tap_build_count++; - build[handle].pid = pid; - build[handle].tag_env = enif_alloc_env(); - build[handle].tag = enif_make_copy(build[handle].tag_env, tag_term); + int slot = tap_build_count++; + int handle = mob_encode_event_handle(tap_build_generation, slot); + if (handle < 0) { + tap_build_count--; + enif_mutex_unlock(tap_mutex); + LOGE(@"register_tap: invalid generation %u", tap_build_generation); + return enif_make_int(env, -1); + } + build[slot].pid = pid; + build[slot].tag_env = enif_alloc_env(); + build[slot].tag = enif_make_copy(build[slot].tag_env, tag_term); enif_mutex_unlock(tap_mutex); return enif_make_int(env, handle); @@ -2182,6 +2267,7 @@ static ERL_NIF_TERM nif_register_tap(ErlNifEnv *env, int argc, const ERL_NIF_TER static ERL_NIF_TERM nif_clear_taps(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { enif_mutex_lock(tap_mutex); + tap_build_generation = mob_next_handle_generation(tap_build_generation); // Prepare the INACTIVE (building) table for a fresh frame; leave the active // table intact so concurrent mob_send_* keep resolving the last committed // frame. The freshly built table is swapped in at set_root. @@ -6753,6 +6839,7 @@ static ERL_NIF_TERM nif_webview_go_back(ErlNifEnv *env, int argc, const ERL_NIF_ typedef struct { ErlNifPid pid; + uint32_t generation; int active; } ComponentHandle; @@ -6773,10 +6860,13 @@ static ERL_NIF_TERM nif_register_component(ErlNifEnv *env, int argc, const ERL_N enif_mutex_lock(component_mutex); for (int i = 0; i < MAX_COMPONENT_HANDLES; i++) { if (!component_handles[i].active) { + component_handles[i].generation = + mob_next_handle_generation(component_handles[i].generation); + int handle = mob_encode_event_handle(component_handles[i].generation, i); component_handles[i].pid = pid; component_handles[i].active = 1; enif_mutex_unlock(component_mutex); - return enif_make_tuple2(env, enif_make_atom(env, "ok"), enif_make_int(env, i)); + return enif_make_tuple2(env, enif_make_atom(env, "ok"), enif_make_int(env, handle)); } } enif_mutex_unlock(component_mutex); @@ -6786,11 +6876,18 @@ static ERL_NIF_TERM nif_register_component(ErlNifEnv *env, int argc, const ERL_N static ERL_NIF_TERM nif_deregister_component(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { int handle; - if (!enif_get_int(env, argv[0], &handle) || handle < 0 || handle >= MAX_COMPONENT_HANDLES) + uint32_t generation; + int slot; + if (!enif_get_int(env, argv[0], &handle) || + !mob_decode_event_handle(handle, &generation, &slot)) return enif_make_badarg(env); enif_mutex_lock(component_mutex); - component_handles[handle].active = 0; + if (!component_handles[slot].active || component_handles[slot].generation != generation) { + enif_mutex_unlock(component_mutex); + return enif_make_badarg(env); + } + component_handles[slot].active = 0; enif_mutex_unlock(component_mutex); return enif_make_atom(env, "ok"); } @@ -6886,15 +6983,17 @@ static ERL_NIF_TERM nif_resolve_ipv4(ErlNifEnv *env, int argc, const ERL_NIF_TER } void mob_send_component_event(int handle, const char *event, const char *payload_json) { - if (handle < 0 || handle >= MAX_COMPONENT_HANDLES) + uint32_t generation; + int slot; + if (!mob_decode_event_handle(handle, &generation, &slot)) return; enif_mutex_lock(component_mutex); - if (!component_handles[handle].active) { + if (!component_handles[slot].active || component_handles[slot].generation != generation) { enif_mutex_unlock(component_mutex); return; } - ErlNifPid pid = component_handles[handle].pid; + ErlNifPid pid = component_handles[slot].pid; enif_mutex_unlock(component_mutex); ErlNifEnv *env = enif_alloc_env(); diff --git a/test/mob/native_event_handle_test.exs b/test/mob/native_event_handle_test.exs index 6a7e75d..4f4c04c 100644 --- a/test/mob/native_event_handle_test.exs +++ b/test/mob/native_event_handle_test.exs @@ -1,65 +1,109 @@ -# Android event handles cross the Compose/JNI boundary as integers, so host -# tests pin the native source contract while the pure Zig codec covers values. +# Native event handles cross the UI/NIF boundary as integers, so host tests pin +# each platform's source contract while the pure Zig codec covers values. # credo:disable-for-this-file Jump.CredoChecks.VacuousTest defmodule Mob.NativeEventHandleTest do use ExUnit.Case, async: true - @source_path Path.expand("../../android/jni/mob_nif.zig", __DIR__) - @source File.read!(@source_path) + @android_source File.read!(Path.expand("../../android/jni/mob_nif.zig", __DIR__)) + @ios_source File.read!(Path.expand("../../ios/mob_nif.m", __DIR__)) test "Android event handles carry the render generation" do - assert @source =~ ~s|const tap_handle_codec = @import("tap_handle_codec.zig")| - assert @source =~ "var tap_active_generation: u32 = 0" - assert @source =~ "var tap_build_generation: u32 = 0" + assert @android_source =~ ~s|const tap_handle_codec = @import("tap_handle_codec.zig")| + assert @android_source =~ "var tap_table_generations: [2]u32" + assert @android_source =~ "var tap_build_generation: u32 = 0" - assert @source =~ + assert @android_source =~ "tap_build_generation = tap_handle_codec.nextGeneration(tap_build_generation)" - assert @source =~ "tap_handle_codec.encode(tap_build_generation, slot_index)" + assert @android_source =~ "tap_handle_codec.encode(tap_build_generation, slot_index)" + end + + test "iOS event handles carry the render generation" do + assert @ios_source =~ "static uint32_t tap_table_generations[2]" + assert @ios_source =~ "static uint32_t tap_build_generation = 0" + assert @ios_source =~ "tap_build_generation = mob_next_handle_generation" + assert @ios_source =~ "mob_encode_event_handle(tap_build_generation, slot)" end test "active table, count, and generation commit under one lock" do - [_, commit] = String.split(@source, "// Commit the freshly-built tap table:", parts: 2) + [_, commit] = + String.split(@android_source, "// Commit the freshly-built tap table:", parts: 2) + [commit, _] = String.split(commit, "erts.enif_mutex_unlock(tap_mutex);", parts: 2) assert commit =~ "tap_active = 1 - tap_active" assert commit =~ "tap_active_count = tap_build_count" - assert commit =~ "tap_active_generation = tap_build_generation" + assert commit =~ "tap_table_generations[tap_active] = tap_build_generation" end test "all active event-table lookups share generation validation" do - assert @source =~ "fn resolveActiveTapLocked(handle: c_int) ?*TapHandle" + assert @android_source =~ "fn resolveActiveTapLocked(handle: c_int) ?*TapHandle" - assert length(Regex.scan(~r/resolveActiveTapLocked\(handle\)/, @source)) == 3, - "snapTap, mob_set_throttle_config, and throttleCheck must use the shared lookup" + assert length(Regex.scan(~r/resolveActiveTapLocked\(handle\)/, @android_source)) >= 3 - refute @source =~ "handle >= tap_active_count" - refute @source =~ "tap_tables[tap_active][@intCast(handle)]" + refute @android_source =~ "handle >= tap_active_count" + refute @android_source =~ "tap_tables[tap_active][@intCast(handle)]" end test "sender tags are copied before the tap-table lock is released" do - [_, snap] = String.split(@source, "fn snapTap", parts: 2) - [snap, _] = String.split(snap, "/// `{:event, tag}`", parts: 2) + [_, snap] = String.split(@android_source, "fn snapTap", parts: 2) + [snap, _] = String.split(snap, "fn snapChangeTap", parts: 2) {lock, _} = :binary.match(snap, "erts.enif_mutex_lock(tap_mutex)") - {copy, _} = :binary.match(snap, "erts.enif_make_copy(env, h.tag)") + {copy, _} = :binary.match(snap, "copyTap(h, env)") {unlock, _} = :binary.matches(snap, "erts.enif_mutex_unlock(tap_mutex)") |> List.last() assert lock < copy and copy < unlock + assert @android_source =~ "erts.enif_make_copy(env, tap.tag)" assert snap =~ "resolveActiveTapLocked(handle) orelse {" assert length(:binary.matches(snap, "erts.enif_mutex_unlock(tap_mutex)")) == 2 end test "every tag snapshot owns and frees its delivery environment" do - snapshots = Regex.scan(~r/snapTap\(handle, env\)/, @source) + snapshots = Regex.scan(~r/snapTap\(handle, env\)/, @android_source) allocated_snapshots = Regex.scan( ~r/const env = erts\.enif_alloc_env\(\) orelse return;\s+defer erts\.enif_free_env\(env\);\s+const snap = snapTap\(handle, env\) orelse return;/, - @source + @android_source ) - assert length(snapshots) == 9 + assert length(snapshots) == 8 assert length(allocated_snapshots) == length(snapshots) + + assert @android_source =~ + ~r/defer erts\.enif_free_env\(env\);\s+const snap = snapChangeTap\(handle, env\) orelse return;/ + end + + test "change events tolerate only identity-preserving stale handles" do + assert @android_source =~ "fn snapChangeTap(handle: c_int, env: ?*erts.ErlNifEnv) ?TapSnap" + assert @android_source =~ "source.pid.pid != active.pid.pid" + assert @android_source =~ "erts.enif_compare(source.tag, active.tag) != 0" + assert @ios_source =~ "mob_snap_change_tap" + assert @ios_source =~ "source->pid.pid != active->pid.pid" + assert @ios_source =~ "enif_compare(source->tag, active->tag) != 0" + end + + test "iOS copies every routed tag while holding the registry lock" do + [_, snap] = String.split(@ios_source, "static int mob_snap_tap", parts: 2) + [snap, _] = String.split(snap, "static int mob_snap_change_tap", parts: 2) + + {lock, _} = :binary.match(snap, "enif_mutex_lock(tap_mutex)") + {copy, _} = :binary.match(snap, "enif_make_copy(msg_env, active->tag)") + {unlock, _} = :binary.matches(snap, "enif_mutex_unlock(tap_mutex)") |> List.last() + + assert lock < copy and copy < unlock + end + + test "component handles reject callbacks from reused slots on both platforms" do + assert @android_source =~ "component_generations" + assert @android_source =~ "decodeComponentHandle" + assert @ios_source =~ "component_handles[slot].generation" + assert @ios_source =~ "mob_decode_event_handle(handle, &generation, &slot)" + end + + test "native handle rejections are visible in debug logs" do + assert @android_source =~ "rejected stale event handle" + assert @ios_source =~ "rejected stale event handle" end end From 9c393cd9e4f590970e95d64bcbcb59f750e021d3 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sun, 30 Aug 2026 23:06:08 -0600 Subject: [PATCH 3/4] fix(native): preserve delayed event identity Hide handler tables while they are rebuilt and allow identity-matched sheet dismissals to survive a concurrent render. --- AGENTS.md | 6 ++++-- CHANGELOG.md | 11 +++++++---- android/jni/mob_nif.zig | 15 ++++++++++++++- ios/mob_nif.m | 19 ++++++++++++++++++- test/mob/native_event_handle_test.exs | 27 +++++++++++++++++++++++++++ 5 files changed, 70 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9a4a2ea..0efb9af 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -162,8 +162,10 @@ These are the things we've burned ourselves on. Following them isn't optional. can route a callback from an old native tree into the current screen. A sender must also copy the tag into its delivery environment while holding that mutex; the table's `tag_env` may be freed as soon as the lock is released. - Change events alone may cross one render when both retained registrations - have identical PID and tag identity; taps and gestures stay generation-strict. + Change events and animation-delayed dismissals may cross one render when + both retained registrations have identical PID and tag identity; taps and + gestures stay generation-strict. Invalidate the building table's generation + at `clear_taps` so stale lookup never observes a partially rebuilt table. 4. **TDD discipline in mob_dev.** Every new public function gets a test. `mob_dev/CLAUDE.md` makes this explicit. Don't bypass — the tests are how we diff --git a/CHANGELOG.md b/CHANGELOG.md index 05106f8..4efaa6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,15 +14,18 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). - **Stale native callbacks no longer route to replacement handlers.** Android and iOS event handles now carry their render generation, with the handler table, count, and generation committed atomically. Taps and gestures from an - old native tree are rejected; text, toggle, and slider changes may cross one - render only when the retained and current registrations have identical PID - and tag identity, preserving in-flight input without permitting misroutes. + old native tree are rejected; change-family events and animation-delayed + sheet dismissals may cross one render only when the retained and current + registrations have identical PID and tag identity, preserving in-flight + intent without permitting misroutes. Building tables are generation-invalid + until committed, so stale lookup cannot observe partially rebuilt handlers. Event tags are copied while their registry lock is held, closing the iOS and Android environment-lifetime race. Persistent component handles also carry a per-slot generation so callbacks from reclaimed slots cannot reach a new component. Rejections are visible in debug native logs. Generated Android projects must pair this with the companion generator update that keys list - state independently of the full event handle. + state independently of the full event handle. Deregistering an already-stale + component handle now returns `badarg` rather than silently succeeding. ## [0.7.37] - 2026-08-30 diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index 35fe217..e18bb2c 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -1126,6 +1126,18 @@ fn sendEvent(handle: c_int, comptime atom_name: [:0]const u8) void { _ = erts.enif_send(null, &pid, env, msg); } +fn sendIdentityEvent(handle: c_int, comptime atom_name: [:0]const u8) void { + const env = erts.enif_alloc_env() orelse return; + defer erts.enif_free_env(env); + const snap = snapChangeTap(handle, env) orelse return; + const msg = erts.makeTuple(env, .{ + erts.enif_make_atom(env, atom_name.ptr), + snap.tag, + }); + var pid = snap.pid; + _ = erts.enif_send(null, &pid, env, msg); +} + /// `{:change, tag, value}` — used by the three change senders below. The /// value term must originate in the same env we're delivering through. fn sendChange(handle: c_int, value_term: erts.ERL_NIF_TERM) void { @@ -1160,7 +1172,7 @@ pub export fn mob_send_tap(handle: c_int) callconv(.c) void { /// screen down — or silently drops it if the screen has a catch-all, in which /// case the BEAM never learns the sheet closed and can't re-present it (MOB-104). pub export fn mob_send_dismiss(handle: c_int) callconv(.c) void { - sendEvent(handle, "dismiss"); + sendIdentityEvent(handle, "dismiss"); } pub export fn mob_send_change_str(handle: c_int, utf8: [*:0]const u8) callconv(.c) void { @@ -1718,6 +1730,7 @@ export fn nif_clear_taps( erts.enif_mutex_lock(tap_mutex); defer erts.enif_mutex_unlock(tap_mutex); tap_build_generation = tap_handle_codec.nextGeneration(tap_build_generation); + tap_table_generations[1 - tap_active] = 0; // Prepare the INACTIVE (building) table for a fresh frame; leave the active // table intact so concurrent mob_send_* keep resolving the last committed // frame. The freshly built table is swapped in at set_root. diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 0101828..e8a85bb 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -334,6 +334,22 @@ static void mob_send_event(int handle, const char *atom) { enif_free_env(msg_env); } +static void mob_send_identity_event(int handle, const char *atom) { + ErlNifEnv *msg_env = enif_alloc_env(); + if (!msg_env) + return; + TapSnap snap; + if (!mob_snap_change_tap(handle, msg_env, &snap)) { + enif_free_env(msg_env); + return; + } + + mob_note_ui_event(); + ERL_NIF_TERM msg = enif_make_tuple2(msg_env, enif_make_atom(msg_env, atom), snap.tag); + enif_send(NULL, &snap.pid, msg_env, msg); + enif_free_env(msg_env); +} + static void mob_send_focus(int handle) { mob_send_event(handle, "focus"); } @@ -347,7 +363,7 @@ static void mob_send_select(int handle) { mob_send_event(handle, "select"); } static void mob_send_dismiss(int handle) { - mob_send_event(handle, "dismiss"); + mob_send_identity_event(handle, "dismiss"); } // IME composition. Sends {compose, tag, %{text: ..., phase: ...}} where @@ -2268,6 +2284,7 @@ static ERL_NIF_TERM nif_register_tap(ErlNifEnv *env, int argc, const ERL_NIF_TER static ERL_NIF_TERM nif_clear_taps(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { enif_mutex_lock(tap_mutex); tap_build_generation = mob_next_handle_generation(tap_build_generation); + tap_table_generations[1 - tap_active] = 0; // Prepare the INACTIVE (building) table for a fresh frame; leave the active // table intact so concurrent mob_send_* keep resolving the last committed // frame. The freshly built table is swapped in at set_root. diff --git a/test/mob/native_event_handle_test.exs b/test/mob/native_event_handle_test.exs index 4f4c04c..f43cee4 100644 --- a/test/mob/native_event_handle_test.exs +++ b/test/mob/native_event_handle_test.exs @@ -84,6 +84,33 @@ defmodule Mob.NativeEventHandleTest do assert @ios_source =~ "enif_compare(source->tag, active->tag) != 0" end + test "building tables are unmatchable until their generation is committed" do + [_, android_clear] = String.split(@android_source, "export fn nif_clear_taps", parts: 2) + [android_clear, _] = String.split(android_clear, "return erts.ok(env);", parts: 2) + + assert android_clear =~ "tap_table_generations[1 - tap_active] = 0" + + [_, ios_clear] = String.split(@ios_source, "static ERL_NIF_TERM nif_clear_taps", parts: 2) + [ios_clear, _] = String.split(ios_clear, "return enif_make_atom(env, \"ok\");", parts: 2) + + assert ios_clear =~ "tap_table_generations[1 - tap_active] = 0" + end + + test "animation-delayed dismissals use identity-preserving stale handling" do + [_, android_dismiss] = + String.split(@android_source, "pub export fn mob_send_dismiss", parts: 2) + + [android_dismiss, _] = + String.split(android_dismiss, "pub export fn mob_send_change_str", parts: 2) + + assert android_dismiss =~ "sendIdentityEvent(handle, \"dismiss\")" + + [_, ios_dismiss] = String.split(@ios_source, "static void mob_send_dismiss", parts: 2) + [ios_dismiss, _] = String.split(ios_dismiss, "// IME composition", parts: 2) + + assert ios_dismiss =~ "mob_send_identity_event(handle, \"dismiss\")" + end + test "iOS copies every routed tag while holding the registry lock" do [_, snap] = String.split(@ios_source, "static int mob_snap_tap", parts: 2) [snap, _] = String.split(snap, "static int mob_snap_change_tap", parts: 2) From 4bffc8122815cb206ac1c92a114b65247f613025 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Mon, 31 Aug 2026 00:26:32 -0600 Subject: [PATCH 4/4] fix(native): preserve identity across renders --- CHANGELOG.md | 7 +-- android/jni/mob_nif.zig | 50 +++++++++++-------- android/jni/tap_handle_codec.zig | 34 +++++++++++++ ios/mob_nif.m | 70 +++++++++++++++++++-------- test/mob/native_event_handle_test.exs | 32 ++++++++++-- 5 files changed, 144 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4efaa6f..b2c13f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,9 +15,10 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). and iOS event handles now carry their render generation, with the handler table, count, and generation committed atomically. Taps and gestures from an old native tree are rejected; change-family events and animation-delayed - sheet dismissals may cross one render only when the retained and current - registrations have identical PID and tag identity, preserving in-flight - intent without permitting misroutes. Building tables are generation-invalid + sheet dismissals may cross any number of renders while the slot's consecutive + committed registrations retain identical PID and tag identity, preserving + in-flight intent without permitting delivery to a replacement handler. + Building tables are generation-invalid until committed, so stale lookup cannot observe partially rebuilt handlers. Event tags are copied while their registry lock is held, closing the iOS and Android environment-lifetime race. Persistent component handles also carry a diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index e18bb2c..0b6818e 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -966,6 +966,10 @@ const TapHandle = extern struct { pid: erts.ErlNifPid, tag_env: ?*erts.ErlNifEnv, tag: erts.ERL_NIF_TERM, + // First generation in the current consecutive run of identical PID/tag + // registrations at this slot. Identity events can safely outlive the two + // physical tables while their route remains unchanged. + identity_start_generation: u32, // ── Batch 5 throttle state — populated by mob_set_throttle_config ── throttle_ms: c_int, @@ -1048,16 +1052,6 @@ fn resolveActiveTapLocked(handle: c_int) ?*TapHandle { return if (tap.tag_env == null) null else tap; } -fn resolveGenerationTapLocked(handle: c_int) ?*TapHandle { - const decoded = tap_handle_codec.decode(handle) orelse return null; - for (0..tap_tables.len) |table_index| { - if (tap_table_generations[table_index] != decoded.generation) continue; - const tap = &tap_tables[table_index][decoded.slot]; - return if (tap.tag_env == null) null else tap; - } - return null; -} - fn copyTap(tap: *const TapHandle, env: ?*erts.ErlNifEnv) TapSnap { return .{ .pid = tap.pid, .tag = erts.enif_make_copy(env, tap.tag), .seq = tap.seq }; } @@ -1087,11 +1081,6 @@ fn snapChangeTap(handle: c_int, env: ?*erts.ErlNifEnv) ?TapSnap { logd_nif("rejected stale event handle {d}", .{handle}); return null; }; - const source = resolveGenerationTapLocked(handle) orelse { - erts.enif_mutex_unlock(tap_mutex); - logd_nif("rejected stale event handle {d}", .{handle}); - return null; - }; const active = if (decoded.slot < @as(usize, @intCast(tap_active_count))) &tap_tables[tap_active][decoded.slot] else { @@ -1099,9 +1088,11 @@ fn snapChangeTap(handle: c_int, env: ?*erts.ErlNifEnv) ?TapSnap { logd_nif("rejected stale event handle {d}", .{handle}); return null; }; - if (active.tag_env == null or source.pid.pid != active.pid.pid or - erts.enif_compare(source.tag, active.tag) != 0) - { + if (active.tag_env == null or !tap_handle_codec.generationWithinIdentity( + decoded.generation, + active.identity_start_generation, + tap_table_generations[tap_active], + )) { erts.enif_mutex_unlock(tap_mutex); logd_nif("rejected stale event handle {d}", .{handle}); return null; @@ -1648,6 +1639,20 @@ export fn nif_set_root( // handlers into 1 - tap_active, so make that table active now. Events for // the new tree (delivered to Compose just below) resolve against it, and // any send racing this swap sees a complete table on either side. + const previous = &tap_tables[tap_active]; + const build = &tap_tables[1 - tap_active]; + var slot_index: usize = 0; + while (slot_index < @as(usize, @intCast(tap_build_count))) : (slot_index += 1) { + const current = &build[slot_index]; + if (slot_index < @as(usize, @intCast(tap_active_count))) { + const prior = &previous[slot_index]; + if (prior.tag_env != null and prior.pid.pid == current.pid.pid and + erts.enif_compare(prior.tag, current.tag) == 0) + { + current.identity_start_generation = prior.identity_start_generation; + } + } + } tap_active = 1 - tap_active; tap_active_count = tap_build_count; tap_table_generations[tap_active] = tap_build_generation; @@ -1709,11 +1714,16 @@ export fn nif_register_tap( loge_nif("register_tap: invalid generation {d}", .{tap_build_generation}); return erts.enif_make_int(env, -1); }; - tap_build_count += 1; + const tag_env = erts.enif_alloc_env() orelse { + loge_nif("register_tap: unable to allocate tag environment", .{}); + return erts.atom(env, "error"); + }; const slot = &tap_tables[1 - tap_active][slot_index]; slot.pid = pid; - slot.tag_env = erts.enif_alloc_env() orelse return erts.atom(env, "error"); + slot.tag_env = tag_env; slot.tag = erts.enif_make_copy(slot.tag_env, tag_term); + slot.identity_start_generation = tap_build_generation; + tap_build_count += 1; return erts.enif_make_int(env, handle); } diff --git a/android/jni/tap_handle_codec.zig b/android/jni/tap_handle_codec.zig index 85662b5..f1a8c9e 100644 --- a/android/jni/tap_handle_codec.zig +++ b/android/jni/tap_handle_codec.zig @@ -32,6 +32,29 @@ pub fn nextGeneration(generation: u32) u32 { return if (generation == 0 or generation >= max_generation) 1 else generation + 1; } +pub fn generationWithinIdentity( + handle_generation: u32, + identity_start_generation: u32, + active_generation: u32, +) bool { + if (handle_generation == 0 or handle_generation > max_generation or + identity_start_generation == 0 or identity_start_generation > max_generation or + active_generation == 0 or active_generation > max_generation) + { + return false; + } + + return generationAge(active_generation, handle_generation) <= + generationAge(active_generation, identity_start_generation); +} + +fn generationAge(active_generation: u32, prior_generation: u32) u32 { + return if (active_generation >= prior_generation) + active_generation - prior_generation + else + max_generation - prior_generation + active_generation; +} + test "round trips every slot through a positive generation-tagged handle" { for (0..slot_count) |slot| { const handle = encode(42, slot).?; @@ -62,3 +85,14 @@ test "generation wraps to one instead of producing invalid handles" { try std.testing.expectEqual(@as(u32, 1), nextGeneration(max_generation)); try std.testing.expectEqual(@as(u32, 1), nextGeneration(0)); } + +test "identity range spans multiple renders and resets on replacement" { + try std.testing.expect(generationWithinIdentity(10, 10, 15)); + try std.testing.expect(generationWithinIdentity(12, 10, 15)); + try std.testing.expect(generationWithinIdentity(15, 10, 15)); + try std.testing.expect(!generationWithinIdentity(9, 10, 15)); + + try std.testing.expect(generationWithinIdentity(max_generation, max_generation - 1, 2)); + try std.testing.expect(generationWithinIdentity(1, max_generation - 1, 2)); + try std.testing.expect(!generationWithinIdentity(max_generation - 2, max_generation - 1, 2)); +} diff --git a/ios/mob_nif.m b/ios/mob_nif.m index e8a85bb..459aff6 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -77,6 +77,10 @@ void mob_set_startup_error(const char *error) { ErlNifPid pid; ErlNifEnv *tag_env; // persistent env owning tag; NULL when slot is free ERL_NIF_TERM tag; + // First generation in the current consecutive run of identical PID/tag + // registrations at this slot. Identity events can safely outlive the two + // physical tables while their route remains unchanged. + uint32_t identity_start_generation; // ── Batch 5 throttle state — populated by mob_set_throttle_config ── int throttle_ms; // 0 = no throttle (raw firing) @@ -109,6 +113,23 @@ static uint32_t mob_next_handle_generation(uint32_t generation) { return generation == 0 || generation >= MAX_EVENT_GENERATION ? 1 : generation + 1; } +static uint32_t mob_generation_age(uint32_t active_generation, uint32_t prior_generation) { + return active_generation >= prior_generation + ? active_generation - prior_generation + : MAX_EVENT_GENERATION - prior_generation + active_generation; +} + +static int mob_generation_within_identity(uint32_t handle_generation, + uint32_t identity_start_generation, + uint32_t active_generation) { + if (handle_generation == 0 || handle_generation > MAX_EVENT_GENERATION || + identity_start_generation == 0 || identity_start_generation > MAX_EVENT_GENERATION || + active_generation == 0 || active_generation > MAX_EVENT_GENERATION) + return 0; + return mob_generation_age(active_generation, handle_generation) <= + mob_generation_age(active_generation, identity_start_generation); +} + static int mob_encode_event_handle(uint32_t generation, int slot) { if (generation == 0 || generation > MAX_EVENT_GENERATION || slot < 0 || slot >= MAX_TAP_HANDLES) return -1; @@ -140,20 +161,6 @@ static int mob_decode_event_handle(int handle, uint32_t *generation, int *slot) return &tap_handles[slot]; } -static TapHandle *mob_resolve_generation_tap_locked(int handle, int *slot_out) { - uint32_t generation; - int slot; - if (!mob_decode_event_handle(handle, &generation, &slot)) - return NULL; - for (int table = 0; table < 2; table++) { - if (tap_table_generations[table] == generation && tap_tables[table][slot].tag_env) { - *slot_out = slot; - return &tap_tables[table][slot]; - } - } - return NULL; -} - static int mob_snap_tap(int handle, ErlNifEnv *msg_env, TapSnap *snap) { enif_mutex_lock(tap_mutex); TapHandle *active = mob_resolve_active_tap_locked(handle); @@ -173,11 +180,17 @@ static int mob_snap_change_tap(int handle, ErlNifEnv *msg_env, TapSnap *snap) { enif_mutex_lock(tap_mutex); TapHandle *active = mob_resolve_active_tap_locked(handle); if (!active) { - int slot = -1; - TapHandle *source = mob_resolve_generation_tap_locked(handle, &slot); + uint32_t generation; + int slot; + if (!mob_decode_event_handle(handle, &generation, &slot)) { + enif_mutex_unlock(tap_mutex); + LOGD(@"rejected stale event handle %d", handle); + return 0; + } active = slot >= 0 && slot < tap_handle_next ? &tap_handles[slot] : NULL; - if (!source || !active || !active->tag_env || source->pid.pid != active->pid.pid || - enif_compare(source->tag, active->tag) != 0) { + if (!active || !active->tag_env || + !mob_generation_within_identity(generation, active->identity_start_generation, + tap_table_generations[tap_active])) { enif_mutex_unlock(tap_mutex); LOGD(@"rejected stale event handle %d", handle); return 0; @@ -2204,6 +2217,14 @@ static ERL_NIF_TERM nif_set_root(ErlNifEnv *env, int argc, const ERL_NIF_TERM ar // Commit the freshly-built tap table: register_tap wrote this frame's // handlers into 1 - tap_active; make that table active now so events for the // new tree resolve against it (readers see a consistent pair under the lock). + TapHandle *previous = tap_tables[tap_active]; + TapHandle *build = tap_tables[1 - tap_active]; + for (int slot = 0; slot < tap_build_count; slot++) { + if (slot < tap_handle_next && previous[slot].tag_env && + previous[slot].pid.pid == build[slot].pid.pid && + enif_compare(previous[slot].tag, build[slot].tag) == 0) + build[slot].identity_start_generation = previous[slot].identity_start_generation; + } tap_active = 1 - tap_active; tap_handles = tap_tables[tap_active]; tap_handle_next = tap_build_count; @@ -2263,17 +2284,24 @@ static ERL_NIF_TERM nif_register_tap(ErlNifEnv *env, int argc, const ERL_NIF_TER return enif_make_int(env, -1); } TapHandle *build = tap_tables[1 - tap_active]; - int slot = tap_build_count++; + int slot = tap_build_count; int handle = mob_encode_event_handle(tap_build_generation, slot); if (handle < 0) { - tap_build_count--; enif_mutex_unlock(tap_mutex); LOGE(@"register_tap: invalid generation %u", tap_build_generation); return enif_make_int(env, -1); } + ErlNifEnv *tag_env = enif_alloc_env(); + if (!tag_env) { + enif_mutex_unlock(tap_mutex); + LOGE(@"register_tap: unable to allocate tag environment"); + return enif_make_atom(env, "error"); + } build[slot].pid = pid; - build[slot].tag_env = enif_alloc_env(); + build[slot].tag_env = tag_env; build[slot].tag = enif_make_copy(build[slot].tag_env, tag_term); + build[slot].identity_start_generation = tap_build_generation; + tap_build_count++; enif_mutex_unlock(tap_mutex); return enif_make_int(env, handle); diff --git a/test/mob/native_event_handle_test.exs b/test/mob/native_event_handle_test.exs index f43cee4..0a880c9 100644 --- a/test/mob/native_event_handle_test.exs +++ b/test/mob/native_event_handle_test.exs @@ -75,13 +75,17 @@ defmodule Mob.NativeEventHandleTest do ~r/defer erts\.enif_free_env\(env\);\s+const snap = snapChangeTap\(handle, env\) orelse return;/ end - test "change events tolerate only identity-preserving stale handles" do + test "identity events tolerate stale handles across unchanged renders" do assert @android_source =~ "fn snapChangeTap(handle: c_int, env: ?*erts.ErlNifEnv) ?TapSnap" - assert @android_source =~ "source.pid.pid != active.pid.pid" - assert @android_source =~ "erts.enif_compare(source.tag, active.tag) != 0" + assert @android_source =~ "identity_start_generation: u32" + assert @android_source =~ "tap_handle_codec.generationWithinIdentity" + assert @android_source =~ "prior.pid.pid == current.pid.pid" + assert @android_source =~ "erts.enif_compare(prior.tag, current.tag) == 0" assert @ios_source =~ "mob_snap_change_tap" - assert @ios_source =~ "source->pid.pid != active->pid.pid" - assert @ios_source =~ "enif_compare(source->tag, active->tag) != 0" + assert @ios_source =~ "identity_start_generation" + assert @ios_source =~ "mob_generation_within_identity" + assert @ios_source =~ "previous[slot].pid.pid == build[slot].pid.pid" + assert @ios_source =~ "enif_compare(previous[slot].tag, build[slot].tag) == 0" end test "building tables are unmatchable until their generation is committed" do @@ -96,6 +100,24 @@ defmodule Mob.NativeEventHandleTest do assert ios_clear =~ "tap_table_generations[1 - tap_active] = 0" end + test "tap registrations allocate their tag environment before publishing the slot" do + [_, android_register] = String.split(@android_source, "export fn nif_register_tap", parts: 2) + [android_register, _] = String.split(android_register, "// nif_clear_taps/0", parts: 2) + + {android_alloc, _} = :binary.match(android_register, "erts.enif_alloc_env()") + {android_publish, _} = :binary.match(android_register, "tap_build_count += 1") + assert android_alloc < android_publish + + [_, ios_register] = + String.split(@ios_source, "static ERL_NIF_TERM nif_register_tap", parts: 2) + + [ios_register, _] = String.split(ios_register, "// ── NIF: clear_taps/0", parts: 2) + + {ios_alloc, _} = :binary.match(ios_register, "enif_alloc_env()") + {ios_publish, _} = :binary.match(ios_register, "tap_build_count++") + assert ios_alloc < ios_publish + end + test "animation-delayed dismissals use identity-preserving stale handling" do [_, android_dismiss] = String.split(@android_source, "pub export fn mob_send_dismiss", parts: 2)