diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a89c9d..3fade0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,18 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). ## [Unreleased] +### Fixed +- **Tap-handle registry is now double-buffered (Android + iOS) — high-frequency + events no longer drop during a render.** `clear_taps` reset the handle count + to 0 and re-registered every handler in tree order, so a drag/scroll firing + from the UI thread *while* a render rebuilt the table saw a transiently-small + count and a half-built table and got dropped — worse the later a widget + registered (e.g. a `Canvas` after a row of `Button`s). `register_tap` now + builds into the inactive table while readers keep resolving the last committed + one; `set_root` swaps them atomically under `tap_mutex`. A concurrent event + always sees a complete table on either side of the swap. No API change. + Verified on-device (moto, finger-drag canvas). + --- ## [0.7.3] - 2026-06-19 diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index a000fd8..4d0f7c1 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -949,8 +949,17 @@ const ComponentHandle = extern struct { active: c_int, }; -var tap_handles: [MAX_TAP_HANDLES]TapHandle = @splat(std.mem.zeroes(TapHandle)); -var tap_handle_next: c_int = 0; +// Double-buffered tap registry. Readers (mob_send_*) resolve a handle against +// the ACTIVE table; a render frame builds into the INACTIVE table (register_tap) +// and swaps it in atomically at set_root. This closes a race where a high- +// frequency event (drag/scroll) firing *during* a re-render saw a half-rebuilt +// table and got dropped — worse the later a widget registered (e.g. a canvas +// after a row of buttons). With the swap a concurrent send always sees a +// complete table (old or new), never a partial one. +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_build_count: c_int = 0; // handles registered so far into the building table 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 @@ -995,8 +1004,8 @@ const TapSnap = struct { fn snapTap(handle: c_int) ?TapSnap { erts.enif_mutex_lock(tap_mutex); defer erts.enif_mutex_unlock(tap_mutex); - if (handle < 0 or handle >= tap_handle_next) return null; - const h = &tap_handles[@intCast(handle)]; + 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 }; } @@ -1157,8 +1166,8 @@ 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_handle_next) return; - const h = &tap_handles[@intCast(handle)]; + if (handle < 0 or handle >= tap_active_count) return; + const h = &tap_tables[tap_active][@intCast(handle)]; if (h.tag_env == null) return; h.throttle_ms = throttle_ms; h.debounce_ms = debounce_ms; @@ -1174,8 +1183,8 @@ 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_handle_next) return false; - const h = &tap_handles[@intCast(handle)]; + 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 throttle_ms: i32 = if (h.throttle_ms != 0) h.throttle_ms else default_throttle_ms; @@ -1499,6 +1508,12 @@ export fn nif_set_root( g_transition[1] = 'o'; g_transition[2] = 'n'; g_transition[3] = 'e'; + // Commit the freshly-built tap table: register_tap wrote this frame's + // 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. + tap_active = 1 - tap_active; + tap_active_count = tap_build_count; erts.enif_mutex_unlock(tap_mutex); const transition_cstr: [*:0]const u8 = @ptrCast(&transition); @@ -1538,11 +1553,11 @@ export fn nif_register_tap( erts.enif_mutex_lock(tap_mutex); defer erts.enif_mutex_unlock(tap_mutex); - if (tap_handle_next >= @as(c_int, @intCast(MAX_TAP_HANDLES))) return erts.badarg(env); + if (tap_build_count >= @as(c_int, @intCast(MAX_TAP_HANDLES))) return erts.badarg(env); - const handle: c_int = tap_handle_next; - tap_handle_next += 1; - const slot = &tap_handles[@intCast(handle)]; + const handle: c_int = tap_build_count; + tap_build_count += 1; + const slot = &tap_tables[1 - tap_active][@intCast(handle)]; 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); @@ -1561,9 +1576,13 @@ export fn nif_clear_taps( _ = argv; erts.enif_mutex_lock(tap_mutex); defer erts.enif_mutex_unlock(tap_mutex); + // 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. + const build = &tap_tables[1 - tap_active]; var i: usize = 0; - while (i < @as(usize, @intCast(tap_handle_next))) : (i += 1) { - const h = &tap_handles[i]; + while (i < MAX_TAP_HANDLES) : (i += 1) { + const h = &build[i]; if (h.tag_env != null) { erts.enif_free_env(h.tag_env); h.tag_env = null; @@ -1579,7 +1598,7 @@ export fn nif_clear_taps( h.last_y = 0; h.seq = 0; } - tap_handle_next = 0; + tap_build_count = 0; return erts.ok(env); } diff --git a/guides/troubleshooting.md b/guides/troubleshooting.md index 5c1d1e4..3dd2a0a 100644 --- a/guides/troubleshooting.md +++ b/guides/troubleshooting.md @@ -312,6 +312,37 @@ rather than hot-push. --- +## Path-dependency mob: on-device `mob_nif:log` undef / stale beams + +**Symptom:** You depend on mob as a local **path dependency** +(`{:mob, path: "../mob", override: true}`) to test an unreleased framework +change on a device. The app crash-loops at boot — logcat shows the bootstrap's +first `mob_nif:log/1` (or `mob_nif:platform/0`) call returning **`undef`**, even +though the on-device `mob_nif.beam` is present and exports the function and the +native `.so` loaded without a `load_nif` error. + +**Cause:** The path-dep's beams in `_build` were stale or only partially +recompiled, so `mix mob.deploy` pushed an `mob` that disagreed with the boot +script — `mob_nif` wasn't loaded when boot first called it. A coherently-built +mob (the published Hex package, or a path-dep recompiled as its own step) boots +fine from the identical app, which is how you tell this apart from a real +framework regression. + +**Fix:** Recompile the path-dep explicitly *before* deploying, then deploy: + +```bash +mix deps.compile mob --force +mix mob.deploy --native --device +``` + +Also compile with the toolchain whose Elixir matches the on-device runtime +(`mob.exs`'s `elixir_lib`) — building candidate `.exs` with a different Elixir +(e.g. an `-rc` vs the final OTP build) emits `:elixir_quote` calls the device +stdlib lacks, a *separate* on-device `undef`. For the committed project, prefer +the Hex package and use the path-dep only as a transient verification vehicle. + +--- + ## Android: app crashes on first distribution startup **Symptom:** App starts successfully, then crashes 3–5 seconds later. Logcat diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 0e63659..d360488 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -77,8 +77,17 @@ void mob_set_startup_error(const char *error) { uint64_t seq; // monotonic counter per handle } TapHandle; -static TapHandle tap_handles[MAX_TAP_HANDLES]; -static int tap_handle_next = 0; +// Double-buffered tap registry (see android/jni/mob_nif.zig for full rationale). +// `tap_handles`/`tap_handle_next` point at the ACTIVE table + its committed +// count — readers (mob_send_*) keep using them unchanged. A render builds into +// the INACTIVE table via register_tap (tap_build_count) and set_root swaps it in +// atomically under tap_mutex, so a concurrent high-frequency send (drag/scroll) +// never observes a half-rebuilt table. +static TapHandle tap_tables[2][MAX_TAP_HANDLES]; +static int tap_active = 0; +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 ErlNifMutex *tap_mutex = NULL; // Convert mach absolute time to nanoseconds (initialised once). @@ -1682,6 +1691,12 @@ static ERL_NIF_TERM nif_set_root(ErlNifEnv *env, int argc, const ERL_NIF_TERM ar strncpy(transition, g_transition, sizeof(transition) - 1); transition[sizeof(transition) - 1] = 0; strncpy(g_transition, "none", sizeof(g_transition)); + // 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). + tap_active = 1 - tap_active; + tap_handles = tap_tables[tap_active]; + tap_handle_next = tap_build_count; enif_mutex_unlock(tap_mutex); NSString *transitionStr = [NSString stringWithUTF8String:transition]; @@ -1709,14 +1724,15 @@ static ERL_NIF_TERM nif_register_tap(ErlNifEnv *env, int argc, const ERL_NIF_TER } enif_mutex_lock(tap_mutex); - if (tap_handle_next >= MAX_TAP_HANDLES) { + if (tap_build_count >= MAX_TAP_HANDLES) { enif_mutex_unlock(tap_mutex); return enif_make_badarg(env); } - int handle = tap_handle_next++; - tap_handles[handle].pid = pid; - tap_handles[handle].tag_env = enif_alloc_env(); - tap_handles[handle].tag = enif_make_copy(tap_handles[handle].tag_env, tag_term); + 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); enif_mutex_unlock(tap_mutex); return enif_make_int(env, handle); @@ -1726,23 +1742,27 @@ 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); - for (int i = 0; i < tap_handle_next; i++) { - if (tap_handles[i].tag_env) { - enif_free_env(tap_handles[i].tag_env); - tap_handles[i].tag_env = NULL; + // 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. + TapHandle *build = tap_tables[1 - tap_active]; + for (int i = 0; i < MAX_TAP_HANDLES; i++) { + if (build[i].tag_env) { + enif_free_env(build[i].tag_env); + build[i].tag_env = NULL; } // Reset throttle state — slots get reused across renders. - tap_handles[i].throttle_ms = 0; - tap_handles[i].debounce_ms = 0; - tap_handles[i].delta_threshold = 0; - tap_handles[i].leading = 1; - tap_handles[i].trailing = 1; - tap_handles[i].last_emit_ns = 0; - tap_handles[i].last_x = 0; - tap_handles[i].last_y = 0; - tap_handles[i].seq = 0; + build[i].throttle_ms = 0; + build[i].debounce_ms = 0; + build[i].delta_threshold = 0; + build[i].leading = 1; + build[i].trailing = 1; + build[i].last_emit_ns = 0; + build[i].last_x = 0; + build[i].last_y = 0; + build[i].seq = 0; } - tap_handle_next = 0; + tap_build_count = 0; enif_mutex_unlock(tap_mutex); return enif_make_atom(env, "ok"); }