diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index 772b535..8cf856d 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -944,7 +944,12 @@ export fn nif_swipe_xy( // mob_nif_init_state (called from mob_nif.c's nif_load BEAM callback). const MAX_TAP_HANDLES: usize = 256; -const MAX_COMPONENT_HANDLES: usize = 64; +// 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 +// constant in ios/mob_nif.m. Still fixed-size: a growable pool or component +// recycling is a longer-term follow-up, not this fix. +const MAX_COMPONENT_HANDLES: usize = 256; /// Per-tap slot: the registered pid, an optional caller-supplied tag, and /// the throttle state for high-frequency events. tag_env is non-null while @@ -1577,7 +1582,19 @@ export fn nif_register_tap( erts.enif_mutex_lock(tap_mutex); defer erts.enif_mutex_unlock(tap_mutex); - if (tap_build_count >= @as(c_int, @intCast(MAX_TAP_HANDLES))) return erts.badarg(env); + if (tap_build_count >= @as(c_int, @intCast(MAX_TAP_HANDLES))) { + // MOB-100 follow-up: this used to be erts.badarg(env), which + // crashed Mob.Renderer.render/3 (and the whole screen process) the + // same way a full component pool used to crash Mob.ComponentServer + // — an unvirtualized long list or big form with >MAX_TAP_HANDLES + // interactive elements would hit this on every render. Every + // sender goes through snapTap/sendEvent/sendChange, which already + // no-op on an out-of-range handle, so -1 is a safe "no handler + // wired up" sentinel here — the interactive prop silently does + // nothing instead of taking the screen down. + loge_nif("register_tap: pool exhausted (cap={d}) — returning unhandled sentinel", .{MAX_TAP_HANDLES}); + return erts.enif_make_int(env, -1); + } const handle: c_int = tap_build_count; tap_build_count += 1; @@ -1644,8 +1661,14 @@ export fn nif_set_transition( } // nif_register_component/1 — allocate a persistent component handle for -// a Native View pid. Linear scan through MAX_COMPONENT_HANDLES slots; -// fails when all are in use. +// a Native View pid. Linear scan through MAX_COMPONENT_HANDLES slots. +// +// Returns {ok, Handle} on success, {error, component_slots_exhausted} when +// the pool is full — MOB-100: a full pool used to return the same +// erts.badarg(env) as a malformed pid argument, which crashed +// Mob.ComponentServer.init (and, via the unhandled {:error, _} tuple +// unmatched in Mob.Component.ensure_started, the whole screen process) +// instead of failing just the one component that couldn't get a slot. export fn nif_register_component( env: ?*erts.ErlNifEnv, argc: c_int, @@ -1662,10 +1685,10 @@ export fn nif_register_component( if (component_handles[i].active == 0) { component_handles[i].pid = pid; component_handles[i].active = 1; - return erts.enif_make_int(env, @intCast(i)); + return erts.makeTuple(env, .{ erts.atom(env, "ok"), erts.enif_make_int(env, @intCast(i)) }); } } - return erts.badarg(env); + return erts.errorTuple(env, erts.atom(env, "component_slots_exhausted")); } // nif_deregister_component/1 — release a component handle. Slot becomes diff --git a/decisions/2026-08-26-component-pool-trap-exit.md b/decisions/2026-08-26-component-pool-trap-exit.md new file mode 100644 index 0000000..faf589f --- /dev/null +++ b/decisions/2026-08-26-component-pool-trap-exit.md @@ -0,0 +1,66 @@ +# Mob.ComponentServer traps exits so terminate/2 actually runs + +- Date: 2026-08-26 +- Status: accepted + +## Context + +MOB-100 reported native component handle pool exhaustion crashing the +screen process on real devices after a few screens of navigation. Two +causes were already known from the report: a fixed 64-slot pool and a +slot-0/sentinel conflation that leaked slot 0 forever. + +While building a regression test against the actual production stop path +(`Mob.ComponentRegistry.reconcile/2`, which calls +`Process.exit(component_pid, :shutdown)` directly to stop components that +left the tree), the fix uncovered a third, more fundamental bug: +`Mob.ComponentServer` is a plain `GenServer` that never sets +`Process.flag(:trap_exit, true)`. A non-trapping process that receives a +raw exit signal (any reason other than `:normal`, `:kill` is a separate +untrappable case) terminates immediately at the VM level — `terminate/2` +is never invoked. Verified empirically: a plain `use GenServer` with a +custom `terminate/2` never observes the callback fire under +`Process.exit(pid, :shutdown)` from another process, only under +`GenServer.stop/2` or a `{:stop, ...}` return from a callback. + +This means `Mob.ComponentServer.terminate/2` — and therefore +`Mob.ComponentServer`'s `nif.deregister_component/1` call — never ran for +*any* component that left a screen's tree via the normal reconcile path, +not just the ones that happened to land on slot 0. Every screen +navigation leaked every component's native handle. The reported "a few +leftover registrations from prior navigation" was this leak, not an +incidental rounding error. + +## Decision + +`Mob.ComponentServer.init/1` now calls `Process.flag(:trap_exit, true)`. +A new `handle_info({:EXIT, _from, reason}, state)` clause (ordered before +the generic catch-all `handle_info/2`) converts the now-trapped exit +signal into `{:stop, reason, state}`, routing it through GenServer's +normal stop machinery so `terminate/2` runs and the native handle is +correctly released. + +This is standard OTP practice for a process that must run cleanup logic +in response to another process telling it to stop via a raw exit signal +— the same reason supervisors require trap_exit on children whose +`:shutdown` value is a timeout rather than `:brutal_kill`. + +## Consequences + +- `Mob.ComponentServer` is no longer killable by an arbitrary exit + signal from an unrelated process the way a non-trapping process would + be — only `Process.exit(pid, :kill)` (or its own `{:stop, ...}` + returns) can end it now. This is the correct, intended behavior for a + process that owns a native resource needing cleanup; no code in this + repo relied on the old kill-any-exit-signal behavior. +- The register/deregister contract (`:mob_nif.register_component/1` + returning `{:ok, handle} | {:error, :component_slots_exhausted}` + instead of a bare int or badarg) is a breaking change to that NIF's + return shape. Grep confirmed the only call site is + `lib/mob/component_server.ex`; no downstream app code calls the NIF + directly. +- The MAX_COMPONENT_HANDLES bump (64 → 256, both platforms) buys + headroom but is still a fixed pool. A screen rendering hundreds of + list-row native components will eventually hit it again; a growable + pool or component recycling is tracked as a longer-term follow-up, not + addressed here. diff --git a/ios/MobNode.m b/ios/MobNode.m index 4aa33b4..a5c70cb 100644 --- a/ios/MobNode.m +++ b/ios/MobNode.m @@ -37,6 +37,7 @@ - (instancetype)init { _fixedHeight = 0.0; _fillWidth = NO; _cornerRadius = 0.0; + _nativeViewHandle = -1; // -1 = no native component slot assigned (MOB-100) _videoAutoplay = NO; _videoLoop = NO; _videoControls = YES; diff --git a/ios/MobRootView.swift b/ios/MobRootView.swift index f1fd391..3c259f8 100644 --- a/ios/MobRootView.swift +++ b/ios/MobRootView.swift @@ -32,6 +32,11 @@ public final class MobNativeViewRegistry { let factory = factories[name], let props = node.nativeViewProps as? [String: Any] else { return nil } let handle = node.nativeViewHandle + // -1 means the BEAM couldn't get a native component slot (pool + // exhausted — MOB-100). Render nothing rather than a view whose + // events would go nowhere; matches Android's MobNativeViewRegistry + // early-return for the same case. + guard handle >= 0 else { return nil } let send: MobNativeSend = { event, payload in if let data = try? JSONSerialization.data(withJSONObject: payload), let json = String(data: data, encoding: .utf8) { diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 0eade8f..2034b8d 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -2018,7 +2018,18 @@ static ERL_NIF_TERM nif_register_tap(ErlNifEnv *env, int argc, const ERL_NIF_TER enif_mutex_lock(tap_mutex); if (tap_build_count >= MAX_TAP_HANDLES) { enif_mutex_unlock(tap_mutex); - return enif_make_badarg(env); + // MOB-100 follow-up: this used to be enif_make_badarg(env), which + // crashed Mob.Renderer.render/3 (and the whole screen process) the + // same way a full component pool used to crash Mob.ComponentServer + // — an unvirtualized long list or big form with >MAX_TAP_HANDLES + // interactive elements would hit this on every render. Every + // mob_send_* sender already no-ops on an out-of-range handle (see + // mob_send_tap et al. above), so -1 is a safe "no handler wired up" + // sentinel here — the interactive prop silently does nothing + // instead of taking the screen down. + LOGE(@"register_tap: pool exhausted (cap=%d) — returning unhandled sentinel", + MAX_TAP_HANDLES); + return enif_make_int(env, -1); } TapHandle *build = tap_tables[1 - tap_active]; int handle = tap_build_count++; @@ -6139,7 +6150,12 @@ static ERL_NIF_TERM nif_webview_go_back(ErlNifEnv *env, int argc, const ERL_NIF_ // register_component/1 allocates a slot; deregister_component/1 frees it. // mob_send_component_event is called from Swift when the native view fires an event. -#define MAX_COMPONENT_HANDLES 64 +// 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 +// constant in android/jni/mob_nif.zig. Still fixed-size: a growable pool +// or component recycling is a longer-term follow-up, not this fix. +#define MAX_COMPONENT_HANDLES 256 typedef struct { ErlNifPid pid; @@ -6149,6 +6165,12 @@ static ERL_NIF_TERM nif_webview_go_back(ErlNifEnv *env, int argc, const ERL_NIF_ static ComponentHandle component_handles[MAX_COMPONENT_HANDLES]; static ErlNifMutex *component_mutex = NULL; +// Returns {ok, Handle} on success, {error, component_slots_exhausted} when +// the pool is full — MOB-100: a full pool used to return the same +// enif_make_badarg(env) as a malformed pid argument, which crashed +// Mob.ComponentServer.init (and, via the unhandled {:error, _} tuple +// unmatched in Mob.Component.ensure_started, the whole screen process) +// instead of failing just the one component that couldn't get a slot. static ERL_NIF_TERM nif_register_component(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { ErlNifPid pid; if (!enif_get_local_pid(env, argv[0], &pid)) @@ -6160,11 +6182,12 @@ static ERL_NIF_TERM nif_register_component(ErlNifEnv *env, int argc, const ERL_N component_handles[i].pid = pid; component_handles[i].active = 1; enif_mutex_unlock(component_mutex); - return enif_make_int(env, i); + return enif_make_tuple2(env, enif_make_atom(env, "ok"), enif_make_int(env, i)); } } enif_mutex_unlock(component_mutex); - return enif_make_badarg(env); + return enif_make_tuple2(env, enif_make_atom(env, "error"), + enif_make_atom(env, "component_slots_exhausted")); } static ERL_NIF_TERM nif_deregister_component(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { diff --git a/lib/mob/component_server.ex b/lib/mob/component_server.ex index c9567ff..887cc37 100644 --- a/lib/mob/component_server.ex +++ b/lib/mob/component_server.ex @@ -6,6 +6,15 @@ defmodule Mob.ComponentServer do use GenServer require Logger + @default_nif :mob_nif + + # Sentinel for "no native handle assigned" — used both when the platform + # doesn't render natively (:no_render, e.g. in tests) and when the native + # component slot pool is exhausted (MOB-100). Slot 0 is a legitimate pool + # index returned by :mob_nif.register_component/1, so it cannot double as + # this sentinel the way it used to (that conflation leaked slot 0 forever). + @no_handle -1 + @doc "Start a component process (not linked to the caller)." @spec start(keyword()) :: {:ok, pid()} | {:error, term()} def start(opts) do @@ -32,11 +41,23 @@ defmodule Mob.ComponentServer do @impl GenServer def init(opts) do + # MOB-100: Mob.ComponentRegistry.reconcile/2 stops a component that has + # left the tree via Process.exit(pid, :shutdown). A GenServer that isn't + # trapping exits terminates immediately on that signal WITHOUT running + # terminate/2 — the native handle (and, before this fix, the registry + # entry) leaked on every single screen navigation, not just for slot 0. + # Trapping exits turns that signal into a regular {:EXIT, _, reason} + # message (handled below) that goes through the normal {:stop, ...} + # path instead, so terminate/2 — and its deregister_component call — + # actually runs. + Process.flag(:trap_exit, true) + module = opts[:module] id = opts[:id] screen_pid = opts[:screen_pid] props = opts[:props] platform = opts[:platform] + nif = opts[:nif] || @default_nif socket = Mob.Socket.new(module, platform: platform) @@ -44,20 +65,67 @@ defmodule Mob.ComponentServer do {:ok, socket} -> Mob.ComponentRegistry.register(screen_pid, id, module, self()) - handle = - if platform != :no_render do - :mob_nif.register_component(self()) - else - 0 - end + handle = register_native_handle(nif, platform, module, id) - {:ok, %{module: module, socket: socket, screen_pid: screen_pid, id: id, handle: handle}} + {:ok, + %{ + module: module, + socket: socket, + screen_pid: screen_pid, + id: id, + handle: handle, + nif: nif + }} {:error, reason} -> {:stop, reason} end end + defp register_native_handle(_nif, :no_render, _module, _id), do: @no_handle + + # `mix mob.push` hot-deploys a new BEAM onto native code that wasn't + # rebuilt (`mix mob.deploy --native` is a separate, opt-in step) — so a + # native binary older than this fix can still be paired with this BEAM. + # That old binary returns a bare int on success and enif_make_badarg(env) + # (which raises ArgumentError at this call site) on pool exhaustion, + # neither of which matches the current {:ok, _} / {:error, _} contract. + # Degrade to no-handle instead of crashing the screen process over a + # version-skew mismatch — the exact failure class this fix exists to + # eliminate. + defp register_native_handle(nif, _platform, module, id) do + case nif.register_component(self()) do + {:ok, handle} when is_integer(handle) -> + handle + + {:error, :component_slots_exhausted} -> + Logger.error( + "[mob_component_server] native component slot pool exhausted — " <> + "#{inspect(module)} id=#{inspect(id)} will not receive native events" + ) + + @no_handle + + other -> + Logger.error( + "[mob_component_server] unexpected register_component/1 return: " <> + "#{inspect(other)} — #{inspect(module)} id=#{inspect(id)} will not " <> + "receive native events (stale native binary? run mix mob.deploy --native)" + ) + + @no_handle + end + rescue + ArgumentError -> + Logger.error( + "[mob_component_server] register_component/1 raised (stale native binary?) — " <> + "#{inspect(module)} id=#{inspect(id)} will not receive native events " <> + "(run mix mob.deploy --native to rebuild)" + ) + + @no_handle + end + @impl GenServer def handle_call(:render_props, _from, %{module: module, socket: socket} = state) do {:reply, module.render(socket.assigns), state} @@ -97,6 +165,13 @@ defmodule Mob.ComponentServer do {:noreply, %{state | socket: new_socket}} end + # Trapping exits (see init/1) turns Mob.ComponentRegistry.reconcile/2's + # Process.exit(pid, :shutdown) into this message instead of an untrappable + # kill — route it through the normal stop path so terminate/2 runs. + def handle_info({:EXIT, _from, reason}, state) do + {:stop, reason, state} + end + def handle_info( message, %{module: module, socket: socket, screen_pid: screen_pid, id: id} = state @@ -162,10 +237,11 @@ defmodule Mob.ComponentServer do socket: socket, screen_pid: screen_pid, id: id, - handle: handle + handle: handle, + nif: nif }) do Mob.ComponentRegistry.deregister(screen_pid, id, module) - if handle != 0, do: :mob_nif.deregister_component(handle) + if handle >= 0, do: nif.deregister_component(handle) module.terminate(reason, socket) end end diff --git a/test/mob/component_server_test.exs b/test/mob/component_server_test.exs index 18817be..8d8a754 100644 --- a/test/mob/component_server_test.exs +++ b/test/mob/component_server_test.exs @@ -1,5 +1,6 @@ defmodule Mob.ComponentServerTest do use ExUnit.Case, async: true + import ExUnit.CaptureLog # Mob.ComponentServer.dispatch/3 (the programmatic Elixir API) bypasses the # native pipeline entirely — payload arrives as an already-decoded map, not @@ -135,6 +136,243 @@ defmodule Mob.ComponentServerTest do end end + describe "native handle registration (MOB-100)" do + # A mock :mob_nif backend so these tests can exercise the + # register_component/deregister_component contract without a device. + # Agent (not GenServer), unlinked, so it survives across test process + # boundaries the same way test/mob/renderer_test.exs's MockNIF does. + defmodule MockNIF do + use Agent + + def start_link, + do: + Agent.start(fn -> %{calls: [], next: 0, freed: [], result: :allocate} end, + name: __MODULE__ + ) + + def calls, do: Agent.get(__MODULE__, & &1.calls) + + def reset, + do: + Agent.update(__MODULE__, fn _ -> %{calls: [], next: 0, freed: [], result: :allocate} end) + + # :allocate — a real freelist pool: reuse a freed slot before growing. + # :exhausted — always report the pool full, like the real pool at capacity. + # :legacy_int / :legacy_badarg — simulate a native binary older than + # MOB-100 (mix mob.push can hot-deploy this BEAM onto native code that + # wasn't rebuilt with `mix mob.deploy --native`): the pre-fix contract + # returned a bare int on success and raised (enif_make_badarg) on + # exhaustion, neither of which matches {:ok, _} / {:error, _}. + def set_result(result), do: Agent.update(__MODULE__, &%{&1 | result: result}) + + def register_component(pid) do + # :legacy_badarg must raise in the CALLING process (matching a real + # NIF's enif_make_badarg), not inside this Agent's own process — + # so the Agent only ever returns a marker; the raise happens below, + # back in the caller. + case Agent.get_and_update(__MODULE__, fn s -> + calls = [{:register_component, [pid]} | s.calls] + + case s.result do + :allocate -> + case s.freed do + [handle | rest] -> {{:ok, handle}, %{s | calls: calls, freed: rest}} + [] -> {{:ok, s.next}, %{s | calls: calls, next: s.next + 1}} + end + + :exhausted -> + {{:error, :component_slots_exhausted}, %{s | calls: calls}} + + :legacy_int -> + {s.next, %{s | calls: calls, next: s.next + 1}} + + :legacy_badarg -> + {:legacy_badarg_marker, %{s | calls: calls}} + end + end) do + :legacy_badarg_marker -> raise ArgumentError, "argument error" + other -> other + end + end + + def deregister_component(handle) do + Agent.update(__MODULE__, fn s -> + %{s | calls: [{:deregister_component, [handle]} | s.calls], freed: [handle | s.freed]} + end) + + :ok + end + end + + setup do + case start_supervised({Mob.ComponentRegistry, []}) do + {:ok, _pid} -> :ok + {:error, {:already_started, _pid}} -> :ok + end + + # Unlinked, fixed-name Agent (mirrors test/mob/renderer_test.exs's + # MockNIF) — reset rather than restarted, since a prior test in this + # module may have left it running. + case MockNIF.start_link() do + {:ok, _pid} -> :ok + {:error, {:already_started, _pid}} -> :ok + end + + MockNIF.reset() + :ok + end + + test ":no_render never calls the native pool and gets the sentinel handle" do + {:ok, pid} = + Mob.ComponentServer.start( + module: Recorder, + id: :norender, + screen_pid: self(), + props: %{}, + platform: :no_render, + nif: MockNIF + ) + + assert Mob.ComponentServer.get_handle(pid) == -1 + assert MockNIF.calls() == [] + + Process.exit(pid, :shutdown) + # terminate/2 runs asynchronously relative to exit; give it a beat. + Process.sleep(10) + assert MockNIF.calls() == [] + end + + test "slot 0 is a valid handle and is deregistered on terminate (no more leak)" do + {:ok, pid} = + Mob.ComponentServer.start( + module: Recorder, + id: :slot0, + screen_pid: self(), + props: %{}, + platform: :ios, + nif: MockNIF + ) + + assert Mob.ComponentServer.get_handle(pid) == 0 + + Process.exit(pid, :shutdown) + Process.sleep(10) + assert {:deregister_component, [0]} in MockNIF.calls() + end + + test "pool exhaustion fails only that component — process survives with the sentinel handle" do + MockNIF.set_result(:exhausted) + + log = + capture_log(fn -> + {:ok, pid} = + Mob.ComponentServer.start( + module: Recorder, + id: :exhausted, + screen_pid: self(), + props: %{}, + platform: :ios, + nif: MockNIF + ) + + assert Process.alive?(pid) + assert Mob.ComponentServer.get_handle(pid) == -1 + + # Still fully functional as an Elixir process — exhaustion only + # costs native rendering, not the component's own state/events. + send(pid, {:component_event, "tapped", "{}"}) + assert_receive {:component_changed, :exhausted, Recorder} + + Process.exit(pid, :shutdown) + Process.sleep(10) + end) + + assert log =~ "component slot pool exhausted" + refute {:deregister_component, [-1]} in MockNIF.calls() + end + + test "a pre-MOB-100 native binary's bare-int return degrades instead of crashing" do + MockNIF.set_result(:legacy_int) + + log = + capture_log(fn -> + {:ok, pid} = + Mob.ComponentServer.start( + module: Recorder, + id: :legacy_int, + screen_pid: self(), + props: %{}, + platform: :ios, + nif: MockNIF + ) + + assert Process.alive?(pid) + assert Mob.ComponentServer.get_handle(pid) == -1 + end) + + assert log =~ "unexpected register_component/1 return" + end + + test "a pre-MOB-100 native binary raising badarg on exhaustion degrades instead of crashing" do + MockNIF.set_result(:legacy_badarg) + + log = + capture_log(fn -> + {:ok, pid} = + Mob.ComponentServer.start( + module: Recorder, + id: :legacy_badarg, + screen_pid: self(), + props: %{}, + platform: :ios, + nif: MockNIF + ) + + assert Process.alive?(pid) + assert Mob.ComponentServer.get_handle(pid) == -1 + end) + + assert log =~ "register_component/1 raised" + end + + test "register/reconcile/register cycling does not leak slots (MOB-100 root cause)" do + # Exercises the REAL production stop path: Mob.ComponentRegistry.reconcile/2 + # calls Process.exit(pid, :shutdown) directly (see lib/mob/component_registry.ex), + # not GenServer.stop. Before trap_exit was added to init/1, that signal + # terminated the process without ever running terminate/2 — so every + # screen navigation leaked a slot, independent of the slot-0 sentinel bug. + screen_pid = self() + + for i <- 1..5 do + id = :"cycled_#{i}" + + {:ok, pid} = + Mob.ComponentServer.start( + module: Recorder, + id: id, + screen_pid: screen_pid, + props: %{}, + platform: :ios, + nif: MockNIF + ) + + # A real pool with a working freelist hands the same slot back out + # every time — proof there's no monotonic growth across cycles. + assert Mob.ComponentServer.get_handle(pid) == 0 + + Mob.ComponentRegistry.reconcile(screen_pid, MapSet.new()) + + # reconcile/2 exits the process; wait for it to actually be gone + # before the next cycle re-registers under the same {screen_pid, id}. + ref = Process.monitor(pid) + assert_receive {:DOWN, ^ref, :process, ^pid, _reason}, 500 + end + + assert Enum.count(MockNIF.calls(), &match?({:register_component, _}, &1)) == 5 + assert Enum.count(MockNIF.calls(), &match?({:deregister_component, _}, &1)) == 5 + end + end + describe "decode_payload/1" do test "decodes a binary JSON map" do assert Mob.ComponentServer.decode_payload(~s({"a":1})) == %{"a" => 1} diff --git a/test/mob/native_component_examples_test.exs b/test/mob/native_component_examples_test.exs index bed6dc6..d31c6db 100644 --- a/test/mob/native_component_examples_test.exs +++ b/test/mob/native_component_examples_test.exs @@ -635,7 +635,7 @@ defmodule Mob.NativeComponentExamplesTest do @node String.to_atom(System.get_env("MOB_TEST_NODE", "mob_demo_ios@127.0.0.1")) describe "[on_device] ComponentServer lifecycle" do - test "register_component allocates a non-zero NIF handle" do + test "register_component allocates a valid NIF handle" do platform = :rpc.call(@node, Application, :get_env, [:mob, :platform, :ios]) {:ok, pid} = @@ -650,7 +650,9 @@ defmodule Mob.NativeComponentExamplesTest do ]) handle = :rpc.call(@node, Mob.ComponentServer, :get_handle, [pid]) - assert is_integer(handle) and handle != 0 + # Slot 0 is a legitimate handle (MOB-100) — it's no longer the + # "not assigned" sentinel, so this only rules out the actual sentinel. + assert is_integer(handle) and handle >= 0 :rpc.call(@node, Process, :exit, [pid, :shutdown]) end diff --git a/test/mob/renderer_test.exs b/test/mob/renderer_test.exs index 24e245b..95de493 100644 --- a/test/mob/renderer_test.exs +++ b/test/mob/renderer_test.exs @@ -10,10 +10,20 @@ defmodule Mob.RendererTest do # Use Agent.start (not start_link) so the Agent is not linked to the test # process and survives across test process boundaries. The setup resets state # rather than restarting the process, eliminating name-registry races. - def start_link, do: Agent.start(fn -> %{calls: [], tap_next: 0} end, name: __MODULE__) + def start_link, + do: + Agent.start(fn -> %{calls: [], tap_next: 0, tap_result: :allocate} end, name: __MODULE__) def calls, do: Agent.get(__MODULE__, & &1.calls) - def reset, do: Agent.update(__MODULE__, fn _ -> %{calls: [], tap_next: 0} end) + + def reset, + do: Agent.update(__MODULE__, fn _ -> %{calls: [], tap_next: 0, tap_result: :allocate} end) + + # :exhausted simulates a full MAX_TAP_HANDLES pool (MOB-100 follow-up) — + # both native sides now return the -1 "unhandled" sentinel instead of + # badarg when the pool is full, since every mob_send_* sender already + # no-ops on an out-of-range handle. + def set_tap_result(result), do: Agent.update(__MODULE__, &%{&1 | tap_result: result}) def clear_taps do Agent.update(__MODULE__, fn s -> @@ -30,9 +40,12 @@ defmodule Mob.RendererTest do def register_tap(pid_or_tagged) do Agent.get_and_update(__MODULE__, fn s -> - handle = s.tap_next calls = [{:register_tap, [pid_or_tagged]} | s.calls] - {handle, %{s | calls: calls, tap_next: handle + 1}} + + case s.tap_result do + :allocate -> {s.tap_next, %{s | calls: calls, tap_next: s.tap_next + 1}} + :exhausted -> {-1, %{s | calls: calls}} + end end) end @@ -150,6 +163,27 @@ defmodule Mob.RendererTest do assert is_integer(decoded["props"]["on_tap"]) end + test "an exhausted tap pool (-1 sentinel) renders instead of crashing (MOB-100 follow-up)" do + MockNIF.set_tap_result(:exhausted) + + tree = %{ + type: :column, + props: %{}, + children: [ + %{type: :button, props: %{text: "A", on_tap: self()}, children: []}, + %{type: :text_field, props: %{id: "f", on_change: {self(), :changed}}, children: []} + ] + } + + assert {:ok, :json_tree} = Renderer.render(tree, :android, MockNIF) + + {:set_root, [json]} = Enum.find(MockNIF.calls(), fn {f, _} -> f == :set_root end) + decoded = :json.decode(json) + [button, field] = decoded["children"] + assert button["props"]["on_tap"] == -1 + assert field["props"]["on_change"] == -1 + end + test "register_tap is called for each on_tap pid" do pid = self()