From 5b43c6627eaa069011c5203cf529b58047e58cfa Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 28 Aug 2026 15:55:08 -0600 Subject: [PATCH 1/5] MOB-112: one process per screen, owned and monitored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single Mob.Screen process held {module, socket, nav, render_mode} and swapped the first two in place on navigation. Every screen shared one mailbox, so a crash in any handle_event took down navigation and every other screen with it. The moduledoc claimed the opposite for a long time; mob#76 corrected the docs, which documented the gap rather than closing it. Mob.Screen.Server is now one process per live screen, owning that screen's socket. Mob.Screen becomes the owner: it holds the navigation state, starts and stops screens, monitors them, and keeps the :mob_screen registered name so the native layer's enif_whereis_pid lookups are unaffected. Its state is the same shape it was, with the socket replaced by the pid of the process that now owns it — Mob.Nav needed no change, because it always treated entries as opaque. Screens are started unlinked and monitored, not linked. The first cut used start_link and the tests caught it immediately: the owner stops a popped screen with GenServer.stop(:shutdown), the link propagated that exit to the owner, and the owner died — taking navigation and every sibling with it. Exactly the coupling this step removes, reintroduced by the mechanism meant to manage it. Not a DynamicSupervisor: a supervisor restarting a screen produces a process the owner knows nothing about, in a slot the supervisor cannot know. The crashed screen might be current, in the active stack's history, or parked under an inactive tab, and restoring it means putting the new pid back exactly where the old one was. The owner is the only thing that knows that. Every owner-to-screen call goes through safe_call/1. dispatch/3 is a call on the owner which calls the screen, so a crash in handle_event would have come back up that call and killed the owner — defeating the isolation on the one path the acceptance criteria name explicitly. A restart re-mounts and loses assigns, logged at error because it is visible to the user. Popping stops the screen leaving the stack; the ones below stay resident, which is what makes pop restore prior state without re-mounting. Demonitoring before stopping matters: otherwise the shutdown the owner asked for returns as a :DOWN and the screen is "restarted" right after being deliberately discarded. A callback that sets a nav action hands it to the owner and does NOT paint — painting there would flash the outgoing tree for a frame before the navigation replaced it, the same overlap that produced MOB-103. Only the active screen may drive navigation, so a background timer cannot yank the stack out from under the user. self() inside a screen is now the screen's own pid, which is what user code always assumed when writing on_tap: {self(), :save} or starting a task. Public API unchanged: dispatch/3, get_socket/1, get_current_module/1 and get_nav_history/1 keep their shapes. Adds get_screen_pid/1, since tooling now needs a way to reach the process actually holding the screen. Rationale in decisions/2026-08-28-screen-processes-and-supervision.md. Device-verified both platforms: cold boot, first frame, tap, a navigation push (the pushed screen renders its own pid — #PID<0.148.0> on iOS, distinct from the owner), pop, and on_change through the new per-screen process. Zero crashes in logcat. Tests: 13 new covering crash isolation end to end. Suite 1214 passed, format and credo --strict clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...-08-28-screen-processes-and-supervision.md | 117 +++ lib/mob/nav.ex | 21 + lib/mob/screen.ex | 865 +++++++++--------- lib/mob/screen/server.ex | 309 +++++++ test/mob/screen/isolation_test.exs | 193 ++++ 5 files changed, 1052 insertions(+), 453 deletions(-) create mode 100644 decisions/2026-08-28-screen-processes-and-supervision.md create mode 100644 lib/mob/screen/server.ex create mode 100644 test/mob/screen/isolation_test.exs diff --git a/decisions/2026-08-28-screen-processes-and-supervision.md b/decisions/2026-08-28-screen-processes-and-supervision.md new file mode 100644 index 00000000..dca3e768 --- /dev/null +++ b/decisions/2026-08-28-screen-processes-and-supervision.md @@ -0,0 +1,117 @@ +# Screen processes: one per screen, owned and monitored rather than supervised + +- Date: 2026-08-28 +- Status: accepted +- Implements: MOB-112, fourth step of MOB-108 +- Builds on: `2026-08-27-screen-process-architecture.md` + +## Context + +One `Mob.Screen` process held `{module, socket, nav, render_mode}` and swapped +the first two in place on navigation. Every screen shared one mailbox, so a +crash in any `handle_event` took down navigation and every other screen with +it. `Mob.Screen`'s moduledoc claimed the opposite for a long time; mob#76 +corrected the docs, which documented the gap rather than closing it. + +## Decision + +`Mob.Screen.Server` is one process per live screen, owning that screen's +socket. `Mob.Screen` becomes the owner: it holds the navigation state, starts +and stops screens, and keeps the `:mob_screen` registered name so the native +layer's `enif_whereis_pid` lookups are unaffected. + +The owner's state is the same shape it was — `{module, socket, nav, +render_mode}` — with the socket replaced by the pid of the process that now +owns it. `Mob.Nav` needed no change to hold pids: it always treated entries as +opaque. + +### Screens are unlinked and monitored, not linked + +The first cut used `start_link`, and the tests caught it immediately: the owner +stops a popped screen with `GenServer.stop(pid, :shutdown)`, the link +propagated that exit to the owner, and the owner died — taking navigation and +every sibling screen with it. Exactly the coupling this step exists to remove, +reintroduced by the mechanism meant to manage it. + +Screens are started with `GenServer.start/2` and monitored. The owner observes +every exit without sharing its fate, and its `terminate/2` stops the screens it +owns so each gets its own `terminate/2` and final state dump. + +### Not a DynamicSupervisor + +A supervisor restarting a screen would produce a process the owner knows +nothing about, in a slot the supervisor cannot know — the crashed screen might +be the current one, in the active stack's history, or parked under an inactive +tab, and restoring it means putting the new pid back exactly where the old one +was. The owner is the only thing that knows that, so it owns the restart. This +is the "deliberate restart strategy" MOB-112 asks for. + +The cost: a screen orphans if the owner is killed without running `terminate/2` +(`Process.exit(owner, :kill)`). Acceptable — the owner dying means the app is +going down — but a `DynamicSupervisor` under a real supervision tree would +close it, and mob does not have one yet. + +### A crash must not come back up a call + +`Mob.Screen.dispatch/3` is a call on the owner, which calls the screen. A crash +in `handle_event` exits that inner call, which would have killed the owner — +defeating the isolation on the one path MOB-112's acceptance names explicitly. +Every owner-to-screen call goes through `safe_call/1`, which catches the exit +and lets the monitor repair the screen. + +### A restart re-mounts, and says so + +A restarted screen runs `mount/3` again and loses its assigns; persisted +screens get their dumped state back through `load_state/2`. This is logged at +error, because it is visible to the user — a form clears, a list resets — and +silently losing state is worse than saying why. + +Background screens (in the active stack's history, or parked under another +stack) are re-mounted eagerly rather than lazily. A crash is rare, and keeping +every nav entry a live pid means popping or switching back never has to handle +a corpse. + +### Popping stops the screen; the ones below stay resident + +The screen leaving the stack is destroyed with it. The screens still in the +history stay alive, which is what makes pop restore prior state without +re-mounting — and what the epic's ADR called out as the memory cost of matching +how iOS and Android actually behave. + +Demonitoring before stopping matters: without it, the shutdown the owner asked +for returns as a `:DOWN` and the screen is "restarted" immediately after being +deliberately discarded. + +### The screen hands navigation to the owner and does not paint + +A callback that sets a nav action has it delivered to the owner, and the screen +does **not** render. Painting there would flash the outgoing screen's tree for a +frame before the navigation replaced it — the same class of overlap that +produced the MOB-103 frame-registry race. + +Two paths, because two guarantees differ. `dispatch/3` is synchronous, so the +screen *returns* the action and the owner applies it before replying. A nav +action from `handle_info` is *sent*, matching the fire-and-forget semantics +`Mob.Test` documents for taps. + +Only the active screen may drive navigation. A background screen's timer must +not yank the stack out from under what the user is looking at. + +## Consequences + +- **`self()` inside a screen is now the screen's own pid.** This is what user + code always assumed when writing `on_tap: {self(), :save}` or starting a + task, and it is what stops screen A's task result being delivered into screen + B's `handle_info` with B's socket (MOB-107). +- Public API is unchanged: `dispatch/3`, `get_socket/1`, `get_current_module/1` + and `get_nav_history/1` all keep their shapes, with the owner fetching + sockets from the screen processes to preserve `[{module, socket}]`. +- `get_screen_pid/1` is added, because tooling now needs a way to reach the + process actually holding the screen. +- `__mob_hot_reload__` became a broadcast: every live screen repaints with the + new code, not just the one on screen. +- Anything else addressed to `:mob_screen` — device events, notifications, + plugin messages — is forwarded by the owner to the active screen. +- An owner-to-screen call returns `nil` rather than raising when the screen is + mid-crash. Callers of `get_socket/1` in that window see `nil`; the monitor + repairs the screen immediately after. diff --git a/lib/mob/nav.ex b/lib/mob/nav.ex index 3b6ec9ee..7c09c701 100644 --- a/lib/mob/nav.ex +++ b/lib/mob/nav.ex @@ -145,6 +145,27 @@ defmodule Mob.Nav do @spec stacks(t()) :: [stack_name()] def stacks(%__MODULE__{order: order}), do: order + @doc """ + Apply `fun` to every parked entry — each inactive stack's current screen and + every entry in its history. + + Entries are opaque to this module, so the caller decides what an entry is and + what replacing one means. `Mob.Screen` uses it to substitute a restarted + screen process wherever it was referenced. + + The *active* stack's history is not covered: it lives in `history`, which the + caller already holds and can rewrite with `put_history/2`. + """ + @spec map_parked(t(), (entry() -> entry())) :: t() + def map_parked(%__MODULE__{parked: parked} = nav, fun) when is_function(fun, 1) do + parked = + Map.new(parked, fn {name, %{current: current, history: history}} -> + {name, %{current: fun.(current), history: Enum.map(history, fun)}} + end) + + %{nav | parked: parked} + end + @doc """ Switch the active stack to `name`, parking `current_entry` under the stack it belongs to. diff --git a/lib/mob/screen.ex b/lib/mob/screen.ex index 59dad848..21fc51d6 100644 --- a/lib/mob/screen.ex +++ b/lib/mob/screen.ex @@ -158,10 +158,21 @@ defmodule Mob.Screen do end end - # ── GenServer wrapper ───────────────────────────────────────────────────── + # ── Owner process ───────────────────────────────────────────────────────── + # + # This process owns navigation: which stacks exist, which is active, and one + # `Mob.Screen.Server` process per live screen. It keeps the `:mob_screen` + # registered name, so the native layer's `enif_whereis_pid` lookups (back + # gesture, alert actions, launch notifications) are unaffected. + # + # Its state mirrors what it held before MOB-112 — `{module, socket, nav, + # render_mode}` — with the socket replaced by the pid of the process that now + # owns it. MOB-113 extracts this role into `Mob.Router`. use GenServer + require Logger + @doc """ Start a screen process linked to the calling process. @@ -177,18 +188,14 @@ defmodule Mob.Screen do Intended for testing and debugging. """ @spec get_current_module(pid()) :: module() - def get_current_module(pid) do - GenServer.call(pid, :get_current_module) - end + def get_current_module(pid), do: GenServer.call(pid, :get_current_module) @doc """ Return the navigation history (list of `{module, socket}` pairs, head = most recent). Intended for testing and debugging. """ @spec get_nav_history(pid()) :: [{module(), Mob.Socket.t()}] - def get_nav_history(pid) do - GenServer.call(pid, :get_nav_history) - end + def get_nav_history(pid), do: GenServer.call(pid, :get_nav_history) @doc """ Start a screen as the root UI screen. Calls mount, renders the component tree @@ -208,27 +215,31 @@ defmodule Mob.Screen do the event has been processed and the state updated. """ @spec dispatch(pid(), String.t(), map()) :: :ok - def dispatch(pid, event, params) do - GenServer.call(pid, {:event, event, params}) - end + def dispatch(pid, event, params), do: GenServer.call(pid, {:event, event, params}) @doc """ Return the current socket state of a running screen. Intended for testing and debugging — not for production app logic. """ @spec get_socket(pid()) :: socket() - def get_socket(pid) do - GenServer.call(pid, :get_socket) - end + def get_socket(pid), do: GenServer.call(pid, :get_socket) + + @doc """ + Return the pid of the process owning the currently active screen. + + Each live screen is its own process since MOB-112; this is how tooling + reaches the one that is on screen. + """ + @spec get_screen_pid(pid()) :: pid() + def get_screen_pid(pid), do: GenServer.call(pid, :get_screen_pid) # ── GenServer callbacks ─────────────────────────────────────────────────── @impl GenServer def init({screen_module, params, render_mode, platform}) do - socket = Mob.Socket.new(screen_module, platform: platform) - - # Register under :mob_screen so C-layer mob_handle_back() can find us. - # Only in :render mode (production); tests use :no_render and run without a NIF. + # Registered under :mob_screen so the C layer's mob_handle_back() and the + # launch-notification fallback find the owner. Only in :render mode; + # tests use :no_render and run without a NIF. if render_mode == :render do Process.register(self(), :mob_screen) # Renders are casts, so a missing sender would blank the screen silently. @@ -238,46 +249,37 @@ defmodule Mob.Screen do Mob.Listener.ensure_started() end - socket = - if render_mode == :render do - {t, r, b, l} = :mob_nif.safe_area() - Mob.Socket.assign(socket, :safe_area, %{top: t, right: r, bottom: b, left: l}) - else - Mob.Socket.assign(socket, :safe_area, %{top: 0.0, right: 0.0, bottom: 0.0, left: 0.0}) - end - - case screen_module.mount(params, %{}, socket) do - {:ok, mounted_socket} -> - # Restore persisted assigns after mount so mount always runs cleanly. - # safe_area is re-applied from the current socket so a stale device - # inset from storage never wins over the live value. - loaded_socket = maybe_load_state(screen_module, mounted_socket) - - # Seed the stacks this app declared. The screen we just mounted becomes - # the active stack's current screen; every other declared stack stays - # unmounted until first visited. With no declaration (or no registry, as - # in tests) this is an empty single-stack state — the old behaviour. - nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) - Mob.Sender.set_active(Mob.Nav.active_ref(nav)) + # Seed the stacks this app declared. The screen we are about to mount + # becomes the active stack's current screen; every other declared stack + # stays unmounted until first visited. With no declaration (or no registry, + # as in tests) this is an empty single-stack state. + nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) + ref = Mob.Nav.active_ref(nav) + Mob.Sender.set_active(ref) + + state = %{ + current: nil, + nav: nav, + render_mode: render_mode, + platform: platform, + monitors: %{} + } - socket = - if render_mode == :render do - # Check for a notification that launched the app from a killed state. - # Send it to self so it arrives via handle_info after init returns, - # consistent with foreground notification delivery. - case :mob_nif.take_launch_notification() do - :none -> :ok - json -> send(self(), {:mob_launch_notification, json}) - end - - do_render(screen_module, loaded_socket, nav) - else - loaded_socket + case start_screen(screen_module, params, ref, state) do + {:ok, entry, state} -> + if render_mode == :render do + # A notification that launched the app from a killed state. Sent to + # self so it arrives via handle_info after init returns, consistent + # with foreground notification delivery. + case :mob_nif.take_launch_notification() do + :none -> :ok + json -> send(self(), {:mob_launch_notification, json}) end - if screen_module.__mob_persist__(), do: schedule_state_sync() + paint(entry, :none, state) + end - {:ok, {screen_module, socket, nav, render_mode}} + {:ok, %{state | current: entry}} {:error, reason} -> {:stop, reason} @@ -285,388 +287,384 @@ defmodule Mob.Screen do end @impl GenServer - def handle_call({:event, event, params}, _from, {module, socket, nav, render_mode}) do - case module.handle_event(event, params, socket) do - {:noreply, new_socket} -> - {module, new_socket, nav, transition} = - apply_nav_action(module, new_socket, nav) - - new_socket = - if render_mode == :render do - do_render_sync(module, new_socket, nav, transition) - else - new_socket - end - - {:reply, :ok, {module, new_socket, nav, render_mode}} + def handle_call({:event, event, params}, _from, state) do + {_module, pid} = state.current + + # The whole point of MOB-112: a crash in the user's handle_event must not + # come back up this call and take the owner — and with it navigation and + # every sibling screen — down. The :DOWN that follows restarts the screen. + case safe_call(fn -> Mob.Screen.Server.dispatch(pid, event, params) end) do + {:ok, {:ok, nav_action}} -> {:reply, :ok, apply_nav_action(nav_action, state, :sync)} + {:exit, _reason} -> {:reply, :ok, state} + end + end - {:reply, _response, new_socket} -> - {module, new_socket, nav, transition} = - apply_nav_action(module, new_socket, nav) + def handle_call({:navigate, nav_action}, _from, state) do + {:reply, :ok, apply_nav_action(nav_action, state, :sync)} + end - new_socket = - if render_mode == :render do - do_render_sync(module, new_socket, nav, transition) - else - new_socket - end + def handle_call(:get_socket, _from, state) do + {_module, pid} = state.current - {:reply, :ok, {module, new_socket, nav, render_mode}} + case safe_call(fn -> Mob.Screen.Server.socket(pid) end) do + {:ok, socket} -> {:reply, socket, state} + {:exit, _reason} -> {:reply, nil, state} end end - @doc """ - Apply a navigation action directly. Used by `Mob.Test` to drive navigation - programmatically without needing a UI event. Synchronous — the caller blocks - until the navigation (and re-render, in production mode) completes. - - Valid actions mirror the `Mob.Socket` navigation functions: - - `{:push, dest, params}` — push a new screen - - `{:pop}` — pop to the previous screen - - `{:pop_to, dest}` — pop to a specific screen in history - - `{:pop_to_root}` — pop to the root of the current stack - - `{:reset, dest, params}` — replace the entire nav stack - """ - def handle_call({:navigate, nav_action}, _from, {module, socket, nav, render_mode}) do - socket = Mob.Socket.put_mob(socket, :nav_action, nav_action) - - {new_module, new_socket, new_nav, transition} = - apply_nav_action(module, socket, nav) - - new_socket = - if render_mode == :render do - do_render_sync(new_module, new_socket, new_nav, transition) - else - new_socket - end + def handle_call(:get_screen_pid, _from, state) do + {_module, pid} = state.current + {:reply, pid, state} + end - {:reply, :ok, {new_module, new_socket, new_nav, render_mode}} + def handle_call(:get_current_module, _from, state) do + {module, _pid} = state.current + {:reply, module, state} end - def handle_call(:get_socket, _from, {_module, socket, _nav, _mode} = state) do - {:reply, socket, state} + def handle_call(:get_nav_history, _from, state) do + {:reply, Enum.map(Mob.Nav.history(state.nav), &entry_with_socket/1), state} end - def handle_call(:inspect, _from, {module, socket, nav, _mode} = state) do - tree = module.render(socket.assigns) + def handle_call(:inspect, _from, state) do + {module, pid} = state.current + {:ok, socket} = safe_call(fn -> Mob.Screen.Server.socket(pid) end) info = %{ screen: module, assigns: socket.assigns, - nav_history: Enum.map(Mob.Nav.history(nav), fn {mod, _} -> mod end), - tree: tree + nav_history: Enum.map(Mob.Nav.history(state.nav), fn {mod, _} -> mod end), + tree: module.render(socket.assigns) } {:reply, info, state} end - def handle_call(:get_current_module, _from, {module, _socket, _nav, _mode} = state) do - {:reply, module, state} - end - - def handle_call(:get_nav_history, _from, {_module, _socket, nav, _mode} = state) do - {:reply, Mob.Nav.history(nav), state} - end - - # Notification that launched the app from a killed state. - # Decoded from JSON and re-dispatched as the standard {:notification, map} message. - # Hot-reload trigger sent by mob_dev after a dist push. Re-render with current code. @impl GenServer - def handle_cast(:__mob_hot_reload__, {module, socket, nav, render_mode}) do - new_socket = - if render_mode == :render do - do_render(module, socket, nav) - else - socket - end - - {:noreply, {module, new_socket, nav, render_mode}} + def handle_cast(:__mob_hot_reload__, state) do + # A broadcast, not one cast: every live screen has to repaint with the + # newly loaded code, not just the one on screen. + Enum.each(all_screen_pids(state), &Mob.Screen.Server.hot_reload/1) + {:noreply, state} end @impl GenServer - def handle_info({:mob_launch_notification, json}, {module, socket, nav, render_mode}) do - notif = decode_notification_json(json) - handle_info({:notification, notif}, {module, socket, nav, render_mode}) + # A screen's callback asked to navigate. Only the active screen may drive + # navigation — a background screen's timer must not yank the stack out from + # under whatever the user is looking at. + def handle_info({:nav_action, action, from}, state) do + {_module, current_pid} = state.current + + if from == current_pid do + {:noreply, apply_nav_action(action, state, :async)} + else + {:noreply, state} + end end - # Android file/camera/photo/scan results arrive as {:mob_file_result, event, sub, json_binary}. - # Decode the JSON and re-dispatch as the user-facing event tuple. - def handle_info({:mob_file_result, event, sub, json_binary}, state) do - event_atom = String.to_atom(event) - sub_atom = String.to_atom(sub) + # A screen crashed. This is the isolation MOB-112 exists to deliver: the + # owner is still here, navigation is intact, and the screen comes back. + def handle_info({:DOWN, _monitor_ref, :process, pid, reason}, state) do + state = %{state | monitors: Map.delete(state.monitors, pid)} + {:noreply, restart_screen(pid, reason, state)} + end - items = - case :json.decode(json_binary) do - list when is_list(list) -> - Enum.map(list, fn item when is_map(item) -> - Map.new(item, fn {k, v} -> {String.to_atom(k), v} end) - end) + # A notification that launched the app from a killed state. + def handle_info({:mob_launch_notification, json}, state) do + handle_info({:notification, decode_notification_json(json)}, state) + end - _ -> - [] + # System back gesture (Android hardware/swipe, iOS edge-pan). Handled here so + # every screen gets back navigation without implementing anything. If a + # WebView is present and has internal history, navigate within it first. + def handle_info({:mob, :back}, state) do + if state.render_mode == :render && :mob_nif.webview_can_go_back() do + :mob_nif.webview_go_back() + {:noreply, state} + else + case {Mob.Nav.history(state.nav), Mob.Nav.back_target(state.nav)} do + {[_ | _], _} -> + {:noreply, apply_nav_action({:pop}, state, :async)} + + # Nothing left to pop on a secondary stack: fall back to the first one + # rather than exiting and discarding every parked stack. + {[], {:switch, target}} -> + {:noreply, apply_nav_action({:switch_tab, target}, state, :async)} + + {[], :exit} -> + if state.render_mode == :render, do: :mob_nif.exit_app() + {:noreply, state} end + end + end - msg = - case {event_atom, sub_atom} do - {:camera, :photo} -> - {:camera, :photo, List.first(items) || %{}} - - {:camera, :video} -> - {:camera, :video, List.first(items) || %{}} - - {:camera, :cancelled} -> - {:camera, :cancelled} - - {:photos, :picked} -> - {:photos, :picked, items} - - {:files, :picked} -> - {:files, :picked, items} + # Anything else addressed to :mob_screen — device events, notifications, + # plugin messages — belongs to the screen the user is looking at. + def handle_info(message, state) do + {_module, pid} = state.current + send(pid, message) + {:noreply, state} + end - {:audio, :recorded} -> - {:audio, :recorded, List.first(items) || %{}} + @impl GenServer + def terminate(_reason, state) do + # Screens are linked, so they come down with us; stopping them explicitly + # is what gives each one its terminate/2 and a final state dump. + Enum.each(all_screen_pids(state), fn pid -> + if Process.alive?(pid), do: GenServer.stop(pid, :shutdown) + end) + end + + # ── Screen lifecycle ────────────────────────────────────────────────────── + + # Calling into a screen that is mid-crash must not propagate into the owner. + # The monitor that follows is what actually repairs the screen. + defp safe_call(fun) do + {:ok, fun.()} + catch + :exit, reason -> {:exit, reason} + end + + defp start_screen(module, params, ref, state) do + opts = [ + module: module, + params: params, + ref: ref, + owner: self(), + render_mode: state.render_mode, + platform: state.platform + ] + + case Mob.Screen.Server.start(opts) do + {:ok, pid} -> + monitor_ref = Process.monitor(pid) + {:ok, {module, pid}, %{state | monitors: Map.put(state.monitors, pid, monitor_ref)}} - {:storage, :saved_to_library} -> - item = List.first(items) || %{} - {:storage, :saved_to_library, item[:path]} + {:error, reason} -> + {:error, reason} + end + end - {:scan, :result} -> - item = List.first(items) || %{} - {:scan, :result, %{type: item[:type] |> to_atom_safe(), value: item[:value]}} + defp all_screen_pids(state) do + current = if state.current, do: [elem(state.current, 1)], else: [] - _ -> - {event_atom, sub_atom, items} - end + parked = + state.nav + |> Map.get(:parked, %{}) + |> Enum.flat_map(fn {_name, %{current: current, history: history}} -> + [current | history] + end) - handle_info(msg, state) + (current ++ + Enum.map(Mob.Nav.history(state.nav), &elem(&1, 1)) ++ Enum.map(parked, &elem(&1, 1))) + |> Enum.uniq() end - # Peripheral.* events: a few carry JSON-encoded device records under tags - # like `:devices_json`, `:permission_granted_json`, etc. The transport's - # own module knows how to decode them; we dispatch through its - # `normalize_message/1` (a no-op for events without JSON payloads) before - # the user's handle_info sees them. - def handle_info({:peripheral, :vendor_usb, _tag, _session, _payload} = msg, state) do - normalized = Mob.VendorUsb.normalize_message(msg) - handle_info(normalized, state) - end + # A crashed screen is re-mounted in place. It loses its assigns — a restart + # runs mount/3 again — which is the documented consequence of the isolation. + defp restart_screen(dead_pid, reason, state) do + ref = Mob.Nav.active_ref(state.nav) - # System back gesture (Android hardware/swipe, iOS edge-pan). - # Handled here — before the user's handle_info — so every screen gets back - # navigation for free without implementing anything. - # If a WebView is present and has internal history, navigate within it first - # before popping the Mob nav stack. - def handle_info({:mob, :back}, {module, socket, nav, render_mode}) do - if render_mode == :render && :mob_nif.webview_can_go_back() do - :mob_nif.webview_go_back() - {:noreply, {module, socket, nav, render_mode}} - else - {module, new_socket, new_nav, transition} = - case {Mob.Nav.history(nav), Mob.Nav.back_target(nav)} do - {[_ | _], _} -> - apply_nav_action(module, Mob.Socket.put_mob(socket, :nav_action, {:pop}), nav) - - # Nothing left to pop on a secondary stack: fall back to the first one - # rather than exiting and discarding every parked stack. - {[], {:switch, target}} -> - apply_nav_action( - module, - Mob.Socket.put_mob(socket, :nav_action, {:switch_tab, target}), - nav - ) - - {[], :exit} -> - if render_mode == :render, do: :mob_nif.exit_app() - {module, socket, nav, :none} - end + case state.current do + {module, ^dead_pid} -> + log_restart(module, reason) - new_socket = - if render_mode == :render do - do_render(module, new_socket, new_nav, transition) - else - new_socket + case start_screen(module, %{}, ref, state) do + {:ok, entry, state} -> + state = %{state | current: entry} + paint(entry, :none, state) + state + + {:error, _reason} -> + state end - {:noreply, {module, new_socket, new_nav, render_mode}} + _ -> + replace_background_screen(dead_pid, reason, state) end end - # List row selected — intercept before the user's handle_info and convert to - # a plain {:select, id, index} message so screens don't need to know about - # the internal {:tap, {:list, ...}} tag format. - def handle_info({:tap, {:list, id, :select, index}}, {module, socket, nav, render_mode}) do - {:noreply, new_socket} = module.handle_info({:select, id, index}, socket) + # A background screen (in the active stack's history, or parked under another + # stack) is re-mounted eagerly rather than lazily. A crash is rare, and + # keeping every entry a live pid means popping or switching back never has to + # deal with a corpse. + defp replace_background_screen(dead_pid, reason, state) do + replace = fn + {module, ^dead_pid} = entry -> + log_restart(module, reason) - {module, new_socket, nav, transition} = - apply_nav_action(module, new_socket, nav) + case start_screen(module, %{}, Mob.Nav.active_ref(state.nav), state) do + {:ok, new_entry, _state} -> new_entry + {:error, _} -> entry + end - new_socket = - if render_mode == :render do - do_render(module, new_socket, nav, transition) - else - new_socket - end + entry -> + entry + end - {:noreply, {module, new_socket, nav, render_mode}} - end + nav = + state.nav + |> Mob.Nav.put_history(Enum.map(Mob.Nav.history(state.nav), replace)) + |> Mob.Nav.map_parked(replace) - # A component's state changed — re-render so the native view gets fresh props. - def handle_info({:component_changed, _id, _module}, {module, socket, nav, render_mode}) do - new_socket = - if render_mode == :render do - do_render(module, socket, nav) - else - socket - end + # Re-monitor whatever came back. Cheap, and it keeps `monitors` honest + # without threading state through the replace function. + monitors = + nav + |> then(fn n -> %{state | nav: n} end) + |> all_screen_pids() + |> Enum.reject(&Map.has_key?(state.monitors, &1)) + |> Map.new(&{&1, Process.monitor(&1)}) - {:noreply, {module, new_socket, nav, render_mode}} + %{state | nav: nav, monitors: Map.merge(state.monitors, monitors)} end - # Periodic state sync — intercepted before the user's handle_info so the - # screen module never sees this internal message. - def handle_info(:__mob_sync_state__, {module, socket, nav, render_mode}) do - if module.__mob_persist__() do - Mob.ScreenState.dump(module, socket) - schedule_state_sync() - end - - {:noreply, {module, socket, nav, render_mode}} + defp log_restart(module, reason) do + Logger.error( + "[mob] screen #{inspect(module)} crashed and is being restarted; its assigns are lost. " <> + "Reason: #{inspect(reason)}" + ) end - # Plugin notification routing: the activated plugins' handlers get first crack - # at every `{:notification, payload}`. A plugin whose `:match` matches handles - # it and the host screen does not also see it; an unmatched notification falls - # through to the screen's own `handle_info` like any other message. - def handle_info({:notification, payload} = message, {_module, _socket, _nav, _mode} = state) - when is_map(payload) do - case Mob.Plugins.dispatch_notification(payload) do - :handled -> {:noreply, state} - :unhandled -> forward_to_screen(message, state) + defp entry_with_socket({module, pid}) do + case safe_call(fn -> Mob.Screen.Server.socket(pid) end) do + {:ok, socket} -> {module, socket} + {:exit, _reason} -> {module, nil} end end - def handle_info(message, state), do: forward_to_screen(message, state) - - defp forward_to_screen(message, {module, socket, nav, render_mode}) do - {:noreply, new_socket} = module.handle_info(message, socket) + defp paint(_entry, _transition, %{render_mode: :no_render}), do: :ok + defp paint({_module, pid}, transition, _state), do: Mob.Screen.Server.render(pid, transition) - {module, new_socket, nav, transition} = - apply_nav_action(module, new_socket, nav) + defp paint_sync(_entry, _transition, %{render_mode: :no_render}), do: :ok - new_socket = - if render_mode == :render do - do_render(module, new_socket, nav, transition) - else - new_socket - end - - {:noreply, {module, new_socket, nav, render_mode}} - end + defp paint_sync({_module, pid}, transition, _state), + do: Mob.Screen.Server.render_sync(pid, transition) - defp to_atom_safe(nil), do: :qr - defp to_atom_safe(s) when is_binary(s), do: String.to_atom(s) - defp to_atom_safe(a) when is_atom(a), do: a + defp do_paint(entry, transition, state, :sync), do: paint_sync(entry, transition, state) + defp do_paint(entry, transition, state, :async), do: paint(entry, transition, state) - @impl GenServer - def terminate(reason, {module, socket, _nav, _render_mode}) do - if module.__mob_persist__(), do: Mob.ScreenState.dump(module, socket) - module.terminate(reason, socket) + # Demonitor before stopping, or the shutdown we asked for comes back as a + # :DOWN and the screen gets "restarted" immediately after being discarded. + defp stop_screen({_module, pid}, state) do + {monitor_ref, monitors} = Map.pop(state.monitors, pid) + if monitor_ref, do: Process.demonitor(monitor_ref, [:flush]) + if Process.alive?(pid), do: GenServer.stop(pid, :shutdown) + %{state | monitors: monitors} end # ── Navigation ──────────────────────────────────────────────────────────── - # Inspect the socket's nav_action and execute it, returning - # {new_module, new_socket, new_nav, transition}. - defp apply_nav_action(module, socket, nav) do - history = Mob.Nav.history(nav) - - case socket.__mob__.nav_action do - nil -> - {module, socket, nav, :none} + defp apply_nav_action(nil, state, _mode), do: state - {:push, dest, params} -> - {new_module, mounted} = mount_destination(dest, params, socket) - saved = {module, clear_nav_action(socket)} - {new_module, mounted, Mob.Nav.put_history(nav, [saved | history]), :push} - - {:pop} -> - case history do - [{prev_module, prev_socket} | rest] -> - {prev_module, prev_socket, Mob.Nav.put_history(nav, rest), :pop} - - [] -> - {module, clear_nav_action(socket), nav, :none} - end - - {:pop_to_root} -> - case Enum.reverse(history) do - [{root_module, root_socket} | _] -> - {root_module, root_socket, Mob.Nav.put_history(nav, []), :pop} + defp apply_nav_action({:push, dest, params}, state, mode) do + {new_module, route_params} = resolve_destination(dest) + ref = Mob.Nav.active_ref(state.nav) - [] -> - {module, clear_nav_action(socket), nav, :none} - end + case start_screen(new_module, Map.merge(route_params, params), ref, state) do + {:ok, entry, state} -> + nav = Mob.Nav.put_history(state.nav, [state.current | Mob.Nav.history(state.nav)]) + state = %{state | nav: nav, current: entry} + do_paint(entry, :push, state, mode) + state - {:pop_to, dest} -> - target = resolve_module(dest) + {:error, _reason} -> + state + end + end - case pop_to_module(history, target) do - {:found, prev_module, prev_socket, rest} -> - {prev_module, prev_socket, Mob.Nav.put_history(nav, rest), :pop} + defp apply_nav_action({:pop}, state, mode) do + case Mob.Nav.history(state.nav) do + [previous | rest] -> + # The screen being popped off leaves the stack for good, so its process + # goes with it. The ones still in `rest` stay resident — that is what + # makes pop restore prior state without re-mounting. + state = stop_screen(state.current, state) + state = %{state | nav: Mob.Nav.put_history(state.nav, rest), current: previous} + do_paint(previous, :pop, state, mode) + state - :not_found -> - {module, clear_nav_action(socket), nav, :none} - end + [] -> + state + end + end - {:reset, dest, params} -> - {new_module, mounted} = mount_destination(dest, params, socket) - {new_module, mounted, Mob.Nav.put_history(nav, []), :reset} + defp apply_nav_action({:pop_to_root}, state, mode) do + case Enum.reverse(Mob.Nav.history(state.nav)) do + [root | _] -> + discarded = [state.current | Enum.reject(Mob.Nav.history(state.nav), &(&1 == root))] + state = Enum.reduce(discarded, state, &stop_screen/2) + state = %{state | nav: Mob.Nav.put_history(state.nav, []), current: root} + do_paint(root, :pop, state, mode) + state - {:switch_tab, tab} -> - apply_switch_tab(module, socket, nav, tab) + [] -> + state end end - # Switching stacks parks the current screen — socket and history both — under - # the stack it belongs to, then makes the target stack current. A stack that - # has been visited before is restored without re-mounting, which is the whole - # point: an inactive tab keeps its state. - # - # The transition is `:none`, not `:push` or `:pop`. Those drive the native - # navigation animation, and a tab switch is a swap rather than a move along a - # stack — animating it as a push would slide the incoming tab in from the - # right on iOS. It also keeps `set_transition` to the atoms native already - # understands, so no `.m` or `.zig` change is needed. - defp apply_switch_tab(module, socket, nav, tab) do - current = {module, clear_nav_action(socket)} - - case Mob.Nav.switch(nav, tab, current) do - {:switched, new_nav, {target_module, target_socket}} -> - Mob.Sender.set_active(Mob.Nav.active_ref(new_nav)) - {target_module, target_socket, new_nav, :none} - - {:mount_root, new_nav, root_module} -> - Mob.Sender.set_active(Mob.Nav.active_ref(new_nav)) - {mounted_module, mounted} = mount_destination(root_module, %{}, socket) - {mounted_module, mounted, new_nav, :none} + defp apply_nav_action({:pop_to, dest}, state, mode) do + target = resolve_module(dest) + history = Mob.Nav.history(state.nav) - :noop -> - {module, clear_nav_action(socket), nav, :none} + case pop_to_module(history, target) do + {:found, previous, rest} -> + discarded = [state.current | Enum.take_while(history, &(&1 != previous))] + state = Enum.reduce(discarded, state, &stop_screen/2) + state = %{state | nav: Mob.Nav.put_history(state.nav, rest), current: previous} + do_paint(previous, :pop, state, mode) + state + + :not_found -> + state end end - # Resolve a destination and mount it on a fresh socket, inheriting the current - # screen's safe-area inset. - defp mount_destination(dest, params, socket) do + defp apply_nav_action({:reset, dest, params}, state, mode) do {new_module, route_params} = resolve_destination(dest) - platform = socket.__mob__.platform + ref = Mob.Nav.active_ref(state.nav) + + case start_screen(new_module, Map.merge(route_params, params), ref, state) do + {:ok, entry, state} -> + discarded = [state.current | Mob.Nav.history(state.nav)] + state = Enum.reduce(discarded, state, &stop_screen/2) + state = %{state | nav: Mob.Nav.put_history(state.nav, []), current: entry} + do_paint(entry, :reset, state, mode) + state + + {:error, _reason} -> + state + end + end - new_base = - Mob.Socket.new(new_module, platform: platform) - |> Mob.Socket.assign(:safe_area, socket.assigns.safe_area) + defp apply_nav_action({:switch_tab, tab}, state, mode) do + case Mob.Nav.switch(state.nav, tab, state.current) do + {:switched, nav, entry} -> + # The restored screen already carries the ref of the stack it was + # parked under, which is the one becoming active. + Mob.Sender.set_active(Mob.Nav.active_ref(nav)) + state = %{state | nav: nav, current: entry} + do_paint(entry, :none, state, mode) + state + + {:mount_root, nav, root_module} -> + ref = Mob.Nav.active_ref(nav) + Mob.Sender.set_active(ref) + state = %{state | nav: nav} + + case start_screen(root_module, %{}, ref, state) do + {:ok, entry, state} -> + state = %{state | current: entry} + do_paint(entry, :none, state, mode) + state + + {:error, _reason} -> + state + end - {:ok, mounted} = new_module.mount(Map.merge(route_params, params), %{}, new_base) - {new_module, mounted} + :noop -> + state + end end defp resolve_module(dest) when is_atom(dest) do @@ -681,11 +679,9 @@ defmodule Mob.Screen do defp resolve_destination(dest) when is_atom(dest) do case Code.ensure_loaded(dest) do {:module, ^dest} -> - # dest is a loaded module — use it directly {dest, %{}} _ -> - # dest is a registered screen name atom — look up in registry case Mob.Nav.Registry.lookup_route(dest) do {:ok, module, route_params} -> {module, route_params} @@ -701,19 +697,72 @@ defmodule Mob.Screen do defp pop_to_module([], _target), do: :not_found - defp pop_to_module([{module, socket} | rest], target) do + defp pop_to_module([{module, _pid} = entry | rest], target) do if module == target do - {:found, module, socket, rest} + {:found, entry, rest} else pop_to_module(rest, target) end end - defp clear_nav_action(socket) do - Mob.Socket.put_mob(socket, :nav_action, nil) + # ── Helpers ─────────────────────────────────────────────────────────────── + + @doc false + # Public only so Mob.Screen.Server can reuse it — the decoding belongs with + # the rest of the native-event translation, not duplicated per process. + @spec decode_file_result(String.t(), String.t(), binary()) :: tuple() + def decode_file_result(event, sub, json_binary) do + event_atom = String.to_atom(event) + sub_atom = String.to_atom(sub) + + items = + case :json.decode(json_binary) do + list when is_list(list) -> + Enum.map(list, fn item when is_map(item) -> + Map.new(item, fn {k, v} -> {String.to_atom(k), v} end) + end) + + _ -> + [] + end + + case {event_atom, sub_atom} do + {:camera, :photo} -> + {:camera, :photo, List.first(items) || %{}} + + {:camera, :video} -> + {:camera, :video, List.first(items) || %{}} + + {:camera, :cancelled} -> + {:camera, :cancelled} + + {:photos, :picked} -> + {:photos, :picked, items} + + {:files, :picked} -> + {:files, :picked, items} + + {:audio, :recorded} -> + {:audio, :recorded, List.first(items) || %{}} + + {:storage, :saved_to_library} -> + {:storage, :saved_to_library, (List.first(items) || %{})[:path]} + + {:scan, :result} -> + scan_result(List.first(items) || %{}) + + _ -> + {event_atom, sub_atom, items} + end + end + + defp scan_result(item) do + {:scan, :result, %{type: to_atom_safe(item[:type]), value: item[:value]}} end - # ── Helpers ─────────────────────────────────────────────────────────────── + defp to_atom_safe(nil), do: :qr + defp to_atom_safe(s) when is_binary(s), do: String.to_atom(s) + defp to_atom_safe(a) when is_atom(a), do: a defp decode_notification_json(json) when is_binary(json) do case :json.decode(json) do @@ -726,11 +775,8 @@ defmodule Mob.Screen do data = case Map.get(map, "data") do - d when is_map(d) -> - Map.new(d, fn {k, v} -> {String.to_atom(k), v} end) - - _ -> - %{} + d when is_map(d) -> Map.new(d, fn {k, v} -> {String.to_atom(k), v} end) + _ -> %{} end %{ @@ -745,91 +791,4 @@ defmodule Mob.Screen do %{source: :local, data: %{}} end end - - # ── State persistence ───────────────────────────────────────────────────── - - @state_sync_interval_ms 30_000 - - defp schedule_state_sync do - Process.send_after(self(), :__mob_sync_state__, @state_sync_interval_ms) - end - - defp maybe_load_state(module, socket) do - if module.__mob_persist__() do - case Mob.ScreenState.load(module, socket) do - {:ok, stored_vsn, raw} -> - restored = module.load_state(stored_vsn, raw) - - socket - |> Mob.Socket.assign(restored) - |> Mob.Socket.assign(:safe_area, socket.assigns.safe_area) - - :not_found -> - socket - end - else - socket - end - end - - # ── Render pipeline ─────────────────────────────────────────────────────── - - defp do_render(module, socket, nav, transition \\ :none) do - platform = socket.__mob__.platform - list_renderers = Map.get(socket.__mob__, :list_renderers, %{}) - socket = ensure_safe_area(socket, platform) - - {tree, active_component_keys} = - module.render(socket.assigns) - # Third expansion pass FIRST: pure-Elixir composites may themselves emit - # nodes / native_view components for the later passes. - |> Mob.Composite.expand(self()) - |> Mob.List.expand(list_renderers, self()) - |> Mob.Component.expand(self(), platform) - - Mob.ComponentRegistry.reconcile(self(), active_component_keys) - - # Every render NIF call goes through the sender — the native tap tables - # share one build cursor, so the clear/register/set_root sequence has to be - # serialised through a single process. See Mob.Sender. - # - # Which screen is active is declared by the navigation code (init and - # apply_switch_tab/4), not here. Announcing it on every render would let any - # screen promote itself simply by re-rendering — at MOB-112 a background - # screen's timer would then commit over the foreground one, disarming the - # drop-inactive mechanism this whole step exists to build. - Mob.Sender.render(Mob.Nav.active_ref(nav), tree, platform, :mob_nif, transition) - - # The commit is asynchronous, so there is no token to wait for. Mob.Renderer - # has only ever returned this one constant. - Mob.Socket.put_root_view(socket, :json_tree) - end - - # The synchronous call paths must not reply until the frame is committed — - # Mob.Test's navigation helpers document that guarantee. The handle_info paths - # stay fire-and-forget, which is what leaves the sender free to coalesce them. - defp do_render_sync(module, socket, nav, transition) do - rendered = do_render(module, socket, nav, transition) - # No deadline: rendering was unbounded when it ran inline, and sync/1 is a - # call, so a default 5s timeout would turn a slow frame on a loaded device - # into a dead screen process. - Mob.Sender.sync(:infinity) - rendered - end - - defp ensure_safe_area(socket, platform) do - if Map.has_key?(socket.assigns, :safe_area) do - socket - else - safe_area = - if platform == :ios do - {t, r, b, l} = :mob_nif.safe_area() - %{top: t, right: r, bottom: b, left: l} - else - %{top: 0.0, right: 0.0, bottom: 0.0, left: 0.0} - end - - Mob.Socket.assign(socket, :safe_area, safe_area) - end - end end diff --git a/lib/mob/screen/server.ex b/lib/mob/screen/server.ex new file mode 100644 index 00000000..b14aa911 --- /dev/null +++ b/lib/mob/screen/server.ex @@ -0,0 +1,309 @@ +defmodule Mob.Screen.Server do + @moduledoc """ + One process per live screen, owning that screen's socket. + + Before MOB-112 a single `Mob.Screen` process held `{module, socket, + nav_history, render_mode}` and swapped the first two in place on navigation. + Every screen shared one mailbox, so a crash in any `handle_event` took down + navigation and every other screen with it — the isolation `Mob.Screen`'s + moduledoc claimed and mob#76 had to write around. + + Now `Mob.Screen` owns navigation and starts one of these per live screen. A + crash here kills this screen only; the owner sees the `:DOWN`, restarts it, + and re-renders. + + ## `self()` means what users already assume + + Inside a screen callback `self()` is now the screen's own pid, not the + process registered as `:mob_screen`. Screens already wrote + `on_tap: {self(), :save}` and started tasks expecting exactly that; before, + those resolved to the one shared process, which is what let a task started by + screen A be delivered into screen B's `handle_info` with B's socket + (MOB-107). + + ## A restart re-mounts + + A restarted screen runs `mount/3` again and loses its assigns. Persisted + screens (`use Mob.Screen, vsn: N` or `persist: true`) get their dumped state + back through `load_state/2`; everything else starts fresh. Stated rather than + implied, because it is the visible consequence of the isolation: the screen + survives, its in-memory state does not. + + ## Navigation is not this process's business + + A user callback that sets a nav action — `push_screen/2`, `pop_screen/1`, + `switch_tab/2` — has that action handed to the owner, and this process does + **not** paint. The owner decides which screen is current and tells that + screen to paint; painting here would flash this screen's tree for a frame + before the navigation replaced it. Ordinary messages never reach the owner, + which is what keeps it off the hot path (MOB-113). + """ + + use GenServer + + @state_sync_interval_ms 30_000 + + @typedoc "Which navigation stack this screen belongs to, for addressing renders." + @type render_ref :: atom() + + defstruct [:module, :socket, :render_mode, :ref, :owner] + + @doc """ + Start a screen process. + + `:owner` receives nav actions and monitors this process. `:ref` is the + navigation stack this screen belongs to, used to address its renders at + `Mob.Sender`. + """ + @spec start_link(keyword()) :: GenServer.on_start() + def start_link(opts), do: GenServer.start_link(__MODULE__, opts) + + @doc """ + Start a screen process **unlinked**. + + This is what `Mob.Screen` uses. Linking would defeat the point: the owner + would die with any screen it stopped or that crashed, taking navigation and + every sibling screen with it — exactly the coupling MOB-112 removes. The + owner monitors instead, so it observes the exit without sharing its fate. + """ + @spec start(keyword()) :: GenServer.on_start() + def start(opts), do: GenServer.start(__MODULE__, opts) + + @doc "Run a user event, returning any navigation action it produced." + @spec dispatch(pid(), String.t(), map()) :: {:ok, term() | nil} + def dispatch(pid, event, params), do: GenServer.call(pid, {:event, event, params}) + + @doc "This screen's current socket." + @spec socket(pid()) :: Mob.Socket.t() + def socket(pid), do: GenServer.call(pid, :get_socket) + + @doc "Paint this screen, with the given navigation transition." + @spec render(pid(), atom()) :: :ok + def render(pid, transition \\ :none), do: GenServer.cast(pid, {:render, transition}) + + @doc "Paint and block until the frame has been committed." + @spec render_sync(pid(), atom()) :: :ok + def render_sync(pid, transition \\ :none), do: GenServer.call(pid, {:render_sync, transition}) + + @doc "Tell this screen which stack it now belongs to." + @spec set_ref(pid(), render_ref()) :: :ok + def set_ref(pid, ref), do: GenServer.cast(pid, {:set_ref, ref}) + + @doc "Repaint with the screen module's newly loaded code." + @spec hot_reload(pid()) :: :ok + def hot_reload(pid), do: GenServer.cast(pid, :__mob_hot_reload__) + + # ── GenServer ───────────────────────────────────────────────────────────── + + @impl GenServer + def init(opts) do + module = Keyword.fetch!(opts, :module) + render_mode = Keyword.get(opts, :render_mode, :no_render) + platform = Keyword.get(opts, :platform, :android) + + socket = + module + |> Mob.Socket.new(platform: platform) + |> Mob.Socket.assign(:safe_area, initial_safe_area(render_mode)) + + case module.mount(Keyword.get(opts, :params, %{}), %{}, socket) do + {:ok, mounted} -> + # Restore persisted assigns after mount so mount always runs cleanly. + socket = maybe_load_state(module, mounted) + if module.__mob_persist__(), do: schedule_state_sync() + + {:ok, + %__MODULE__{ + module: module, + socket: socket, + render_mode: render_mode, + ref: Keyword.get(opts, :ref, :__mob_single__), + owner: Keyword.fetch!(opts, :owner) + }} + + {:error, reason} -> + {:stop, reason} + end + end + + @impl GenServer + def handle_call({:event, event, params}, _from, state) do + case state.module.handle_event(event, params, state.socket) do + {:noreply, socket} -> reply_after_callback(socket, state) + {:reply, _payload, socket} -> reply_after_callback(socket, state) + end + end + + def handle_call(:get_socket, _from, state), do: {:reply, state.socket, state} + + def handle_call({:render_sync, transition}, _from, state) do + {:reply, :ok, %{state | socket: paint(state, transition, :sync)}} + end + + @impl GenServer + def handle_cast({:render, transition}, state) do + {:noreply, %{state | socket: paint(state, transition)}} + end + + def handle_cast({:set_ref, ref}, state), do: {:noreply, %{state | ref: ref}} + + def handle_cast(:__mob_hot_reload__, state) do + {:noreply, %{state | socket: paint(state, :none)}} + end + + @impl GenServer + # A list row selection arrives as a tap with a structured tag; the user sees + # the simpler {:select, id, index}. + def handle_info({:tap, {:list, id, :select, index}}, state) do + forward({:select, id, index}, state) + end + + # A component's state changed — repaint so the native view gets fresh props. + def handle_info({:component_changed, _id, _module}, state) do + {:noreply, %{state | socket: paint(state, :none)}} + end + + # Periodic state sync — intercepted before the user's handle_info so the + # screen module never sees this internal message. + def handle_info(:__mob_sync_state__, state) do + if state.module.__mob_persist__() do + Mob.ScreenState.dump(state.module, state.socket) + schedule_state_sync() + end + + {:noreply, state} + end + + # Android file/camera/photo/scan results arrive JSON-encoded; decode and + # re-dispatch as the user-facing event tuple. + def handle_info({:mob_file_result, event, sub, json_binary}, state) do + handle_info(Mob.Screen.decode_file_result(event, sub, json_binary), state) + end + + # A few Peripheral.* events carry JSON-encoded device records; the + # transport's own module knows how to decode them. + def handle_info({:peripheral, :vendor_usb, _tag, _session, _payload} = msg, state) do + handle_info(Mob.VendorUsb.normalize_message(msg), state) + end + + # Activated plugins get first crack at every notification. One whose :match + # matches handles it and the screen never sees it. + def handle_info({:notification, payload} = message, state) when is_map(payload) do + case Mob.Plugins.dispatch_notification(payload) do + :handled -> {:noreply, state} + :unhandled -> forward(message, state) + end + end + + def handle_info(message, state), do: forward(message, state) + + @impl GenServer + def terminate(reason, state) do + if state.module.__mob_persist__(), do: Mob.ScreenState.dump(state.module, state.socket) + state.module.terminate(reason, state.socket) + end + + # ── Internals ───────────────────────────────────────────────────────────── + + defp forward(message, state) do + {:noreply, socket} = state.module.handle_info(message, state.socket) + + case take_nav_action(socket) do + {nil, socket} -> + state = %{state | socket: socket} + {:noreply, %{state | socket: paint(state, :none)}} + + {action, socket} -> + send(state.owner, {:nav_action, action, self()}) + {:noreply, %{state | socket: socket}} + end + end + + defp reply_after_callback(socket, state) do + case take_nav_action(socket) do + {nil, socket} -> + state = %{state | socket: socket} + {:reply, {:ok, nil}, %{state | socket: paint(state, :none, :sync)}} + + {action, socket} -> + # Returned rather than sent: Mob.Screen.dispatch/3 is synchronous, so + # the owner applies the action before replying to its own caller. + {:reply, {:ok, action}, %{state | socket: socket}} + end + end + + defp take_nav_action(socket) do + case socket.__mob__.nav_action do + nil -> {nil, socket} + action -> {action, Mob.Socket.put_mob(socket, :nav_action, nil)} + end + end + + defp paint(state, transition, mode \\ :async) + defp paint(%{render_mode: :no_render} = state, _transition, _mode), do: state.socket + + defp paint(state, transition, mode) do + socket = ensure_safe_area(state.socket, state.socket.__mob__.platform) + platform = socket.__mob__.platform + list_renderers = Map.get(socket.__mob__, :list_renderers, %{}) + + {tree, active_component_keys} = + state.module.render(socket.assigns) + # Third expansion pass FIRST: pure-Elixir composites may themselves emit + # nodes / native_view components for the later passes. + |> Mob.Composite.expand(self()) + |> Mob.List.expand(list_renderers, self()) + |> Mob.Component.expand(self(), platform) + + Mob.ComponentRegistry.reconcile(self(), active_component_keys) + Mob.Sender.render(state.ref, tree, platform, :mob_nif, transition) + if mode == :sync, do: Mob.Sender.sync(:infinity) + + Mob.Socket.put_root_view(socket, :json_tree) + end + + defp initial_safe_area(:render) do + {t, r, b, l} = :mob_nif.safe_area() + %{top: t, right: r, bottom: b, left: l} + end + + defp initial_safe_area(_mode), do: %{top: 0.0, right: 0.0, bottom: 0.0, left: 0.0} + + defp ensure_safe_area(socket, platform) do + if Map.has_key?(socket.assigns, :safe_area) do + socket + else + safe_area = + if platform == :ios do + {t, r, b, l} = :mob_nif.safe_area() + %{top: t, right: r, bottom: b, left: l} + else + %{top: 0.0, right: 0.0, bottom: 0.0, left: 0.0} + end + + Mob.Socket.assign(socket, :safe_area, safe_area) + end + end + + defp maybe_load_state(module, socket) do + if module.__mob_persist__() do + case Mob.ScreenState.load(module, socket) do + {:ok, stored_vsn, raw} -> + restored = module.load_state(stored_vsn, raw) + + socket + |> Mob.Socket.assign(restored) + |> Mob.Socket.assign(:safe_area, socket.assigns.safe_area) + + :not_found -> + socket + end + else + socket + end + end + + defp schedule_state_sync do + Process.send_after(self(), :__mob_sync_state__, @state_sync_interval_ms) + end +end diff --git a/test/mob/screen/isolation_test.exs b/test/mob/screen/isolation_test.exs new file mode 100644 index 00000000..d6fb1b40 --- /dev/null +++ b/test/mob/screen/isolation_test.exs @@ -0,0 +1,193 @@ +defmodule Mob.Screen.IsolationTest do + @moduledoc """ + The crash isolation `Mob.Screen`'s moduledoc claimed for a long time and + mob#76 had to write around. + + Before MOB-112 every screen shared one process, so a crash in any + `handle_event` took navigation and every other screen with it. + """ + use ExUnit.Case, async: false + + import ExUnit.CaptureLog + + defmodule HomeScreen do + use Mob.Screen + + @detail Mob.Screen.IsolationTest.DetailScreen + + def mount(_params, _session, socket), do: {:ok, Mob.Socket.assign(socket, :count, 0)} + def render(assigns), do: %{type: :text, props: %{text: "home #{assigns.count}"}, children: []} + + def handle_event("bump", _, socket), + do: {:noreply, Mob.Socket.assign(socket, :count, socket.assigns.count + 1)} + + def handle_event("boom", _, _socket), do: raise("screen exploded") + + def handle_event("push", _, socket), + do: {:noreply, Mob.Socket.push_screen(socket, @detail)} + + def handle_info(:who_am_i, socket), + do: {:noreply, Mob.Socket.assign(socket, :seen_self, self())} + + def handle_info(_msg, socket), do: {:noreply, socket} + end + + defmodule DetailScreen do + use Mob.Screen + + def mount(_params, _session, socket), do: {:ok, Mob.Socket.assign(socket, :where, :detail)} + def render(assigns), do: %{type: :text, props: %{text: "#{assigns.where}"}, children: []} + + def handle_event("boom", _, _socket), do: raise("detail exploded") + def handle_event("back", _, socket), do: {:noreply, Mob.Socket.pop_screen(socket)} + end + + defmodule DemoApp do + @behaviour Mob.App + import Mob.App + @home Mob.Screen.IsolationTest.HomeScreen + def navigation(_), do: stack(:home, root: @home) + end + + defp history_pids(owner) do + owner |> :sys.get_state() |> Map.fetch!(:nav) |> Mob.Nav.history() |> Enum.map(&elem(&1, 1)) + end + + setup do + case Process.whereis(Mob.Nav.Registry) do + nil -> :ok + pid -> GenServer.stop(pid) + end + + {:ok, registry} = Mob.Nav.Registry.start_link(DemoApp) + on_exit(fn -> if Process.alive?(registry), do: GenServer.stop(registry) end) + + {:ok, owner} = Mob.Screen.start_link(HomeScreen, %{}) + on_exit(fn -> if Process.alive?(owner), do: GenServer.stop(owner) end) + + %{owner: owner} + end + + describe "one process per screen" do + test "the owner and the screen are different processes", %{owner: owner} do + assert Mob.Screen.get_screen_pid(owner) != owner + end + + test "self() inside a callback is the screen's own pid", %{owner: owner} do + # What user code already assumed when writing on_tap: {self(), :tag} or + # starting a task. Before MOB-112 this was the one shared process, which + # is what let screen A's task result land in screen B (MOB-107). + screen = Mob.Screen.get_screen_pid(owner) + send(screen, :who_am_i) + :sys.get_state(screen) + + assert Mob.Screen.get_socket(owner).assigns.seen_self == screen + end + + test "pushing starts a second screen process and keeps the first", %{owner: owner} do + first = Mob.Screen.get_screen_pid(owner) + Mob.Screen.dispatch(owner, "push", %{}) + second = Mob.Screen.get_screen_pid(owner) + + assert second != first + assert Process.alive?(first), "the screen below stays resident so pop restores it" + end + end + + describe "crash isolation" do + test "a crashing handle_event does not take down the owner", %{owner: owner} do + capture_log(fn -> Mob.Screen.dispatch(owner, "boom", %{}) end) + assert Process.alive?(owner) + end + + test "navigation survives a crash", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + capture_log(fn -> Mob.Screen.dispatch(owner, "boom", %{}) end) + + assert length(Mob.Screen.get_nav_history(owner)) == 1 + end + + test "a sibling screen survives a crash", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + [home_pid] = history_pids(owner) + + capture_log(fn -> Mob.Screen.dispatch(owner, "boom", %{}) end) + + assert Process.alive?(home_pid), "the screen below the crash is untouched" + end + + test "the owner restarts the crashed screen", %{owner: owner} do + before = Mob.Screen.get_screen_pid(owner) + capture_log(fn -> Mob.Screen.dispatch(owner, "boom", %{}) end) + :sys.get_state(owner) + + after_crash = Mob.Screen.get_screen_pid(owner) + assert after_crash != before + assert Process.alive?(after_crash) + assert Mob.Screen.get_current_module(owner) == HomeScreen + end + + test "a restarted screen re-mounts and loses its assigns", %{owner: owner} do + Mob.Screen.dispatch(owner, "bump", %{}) + assert Mob.Screen.get_socket(owner).assigns.count == 1 + + capture_log(fn -> Mob.Screen.dispatch(owner, "boom", %{}) end) + :sys.get_state(owner) + + # mount/3 ran again. Documented, not incidental. + assert Mob.Screen.get_socket(owner).assigns.count == 0 + end + + test "the restart is logged, since losing assigns is visible to users", %{owner: owner} do + log = + capture_log(fn -> + Mob.Screen.dispatch(owner, "boom", %{}) + # The restart happens when the :DOWN lands, after dispatch returns. + :sys.get_state(owner) + end) + + assert log =~ "crashed and is being restarted" + end + + test "the screen still works after being restarted", %{owner: owner} do + capture_log(fn -> Mob.Screen.dispatch(owner, "boom", %{}) end) + :sys.get_state(owner) + + Mob.Screen.dispatch(owner, "bump", %{}) + assert Mob.Screen.get_socket(owner).assigns.count == 1 + end + end + + describe "screen lifecycle" do + test "popping stops the screen that leaves the stack", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + detail = Mob.Screen.get_screen_pid(owner) + ref = Process.monitor(detail) + + Mob.Screen.dispatch(owner, "back", %{}) + + assert_receive {:DOWN, ^ref, :process, ^detail, _} + assert Mob.Screen.get_current_module(owner) == HomeScreen + end + + test "a popped screen is not restarted", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + detail = Mob.Screen.get_screen_pid(owner) + Mob.Screen.dispatch(owner, "back", %{}) + :sys.get_state(owner) + + refute Process.alive?(detail) + assert Mob.Screen.get_current_module(owner) == HomeScreen + assert Mob.Screen.get_nav_history(owner) == [] + end + + test "stopping the owner stops its screens", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + detail = Mob.Screen.get_screen_pid(owner) + ref = Process.monitor(detail) + + GenServer.stop(owner) + assert_receive {:DOWN, ^ref, :process, ^detail, _} + end + end +end From 78b81307b9f402b9292e37059696653e7d1e04e3 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 28 Aug 2026 18:34:36 -0600 Subject: [PATCH 2/5] MOB-112: fix eleven defects found by adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two that defeated the step's own acceptance criteria: - handle_call(:inspect) pattern-matched {:ok, socket} from safe_call/1, so the one case safe_call exists for raised a MatchError inside the owner and killed it. Reached by Mob.Test.inspect/1 and tree/1 against a screen that is mid-crash or merely busy — an agent debugging a wedged screen destroyed the app it was debugging. - paint_sync was a raw GenServer.call, so a crash in the user's render/1 exited the owner. That sits on the everyday "tap a button that pushes a screen" path, more common than the handle_event crash the tests did cover. Both are now wrapped. The ADR claimed "every owner-to-screen call goes through safe_call/1"; two did not. Linking took two wrong turns, each fix causing the next problem, so the reasoning is recorded rather than just the result. Linking alone kills the owner when it stops a popped screen. Unlinking fixes that and orphans every screen when the owner dies — and an orphaned persisted screen keeps its 30s timer, dumping to Mob.ScreenState under the same key as its live replacement. The answer is both: linked, with owner AND screens trapping exits. The owner's terminate/2 no longer stops screens, because calling GenServer.stop/3 there lets the screen's :shutdown travel back up the link mid-terminate and replace the owner's own exit reason. Navigation entries carry params and ref, not just {module, pid}: - a screen mounting on %{id: id} cannot come back from %{}; the re-mount raised, the restart failed, and the owner kept a dead pid as `current` — every later event called a corpse and returned :ok, freezing the app with no log after the first line - a parked screen restarted with the *active* ref commits over the foreground tab on its next repaint, undoing MOB-110's drop-inactive mechanism A failed re-mount no longer leaves a corpse: background screens are dropped from their stack, a current screen pops to what is beneath it, and with nothing beneath it says so. Also: every no-op nav branch repaints (the screen deliberately does not paint when it produced an action, so re-tapping the active tab updated assigns that never reached the screen); switch_tab mounts before mutating nav and the sender's active ref, so a failed mount cannot leave the sender addressing a stack whose screen never started; owner-to-screen calls pass :infinity, since a 5s bound silently discarded the navigation the user asked for rather than failing; deliberate stops are bounded so one wedged screen cannot block teardown; and the double-monitor bookkeeping is gone with the monitors. Mob.Test was in this change's blast radius even though it was not in the diff. settle/2 drains three processes now — :mob_screen is the navigation owner, which forwards to the screen, which the sender commits — and assigns/1 tolerates the nil socket a mid-restart screen returns. Corrected the moduledoc, which mob#76 had already had to fix once and which this change made wrong in a new way, and a comment that said screens are linked back when they were not. Tests: 9 new covering background-screen restarts, params and ref preservation, and a two-stack app — the gap that let both defects ship. Verified as negative controls: reinstating either defect fails exactly the test written for it. The three racy `if Process.alive?, do: GenServer.stop` teardowns are exit-safe. Suite 1223 passed, format and credo --strict clean. Re-verified on the iOS simulator with the final process model: cold boot, push (own pid #PID<0.142.0>), pop, zero errors in the BEAM log. Co-Authored-By: Claude Opus 5 (1M context) --- ...-08-28-screen-processes-and-supervision.md | 115 +++++- lib/mob/screen.ex | 360 +++++++++++------- lib/mob/screen/server.ex | 51 +-- lib/mob/test.ex | 37 +- test/mob/nav/multi_stack_test.exs | 13 +- test/mob/screen/isolation_test.exs | 15 +- test/mob/screen/restart_test.exs | 211 ++++++++++ test/mob/screen_sender_wiring_test.exs | 13 +- 8 files changed, 614 insertions(+), 201 deletions(-) create mode 100644 test/mob/screen/restart_test.exs diff --git a/decisions/2026-08-28-screen-processes-and-supervision.md b/decisions/2026-08-28-screen-processes-and-supervision.md index dca3e768..0fdf3dba 100644 --- a/decisions/2026-08-28-screen-processes-and-supervision.md +++ b/decisions/2026-08-28-screen-processes-and-supervision.md @@ -25,17 +25,47 @@ render_mode}` — with the socket replaced by the pid of the process that now owns it. `Mob.Nav` needed no change to hold pids: it always treated entries as opaque. -### Screens are unlinked and monitored, not linked - -The first cut used `start_link`, and the tests caught it immediately: the owner -stops a popped screen with `GenServer.stop(pid, :shutdown)`, the link -propagated that exit to the owner, and the owner died — taking navigation and -every sibling screen with it. Exactly the coupling this step exists to remove, -reintroduced by the mechanism meant to manage it. - -Screens are started with `GenServer.start/2` and monitored. The owner observes -every exit without sharing its fate, and its `terminate/2` stops the screens it -owns so each gets its own `terminate/2` and final state dump. +### Screens are linked, and everyone traps exits + +This took two wrong turns worth recording, because each fix created the next +problem. + +Linking alone is wrong: the owner stops a popped screen with +`GenServer.stop(pid, :shutdown)`, the link propagates that exit, and the owner +dies — taking navigation and every sibling with it. Exactly the coupling this +step removes, reintroduced by the mechanism meant to manage it. + +Unlinking (`GenServer.start/2` plus a monitor) fixes that and is also wrong: it +orphans every screen when the owner dies. That is not hypothetical — an +orphaned screen keeps its 30s `Mob.ScreenState` timer and dumps under the same +`screen_key` as its live replacement, so orphans overwrite live persisted +state. + +The answer is both: screens are **linked**, and the owner **traps exits**. The +owner sees each exit as an `{:EXIT, pid, reason}` message without sharing its +fate, and screens still come down with it. Screens trap too, so gen_server +turns the owner's exit into a `terminate/2` call — which is what runs the +user's `terminate/2` and the final state dump. + +Two consequences fall out: + +* The owner's `terminate/2` deliberately does **not** stop screens. Calling + `GenServer.stop/3` there corrupts the owner's own exit: the screen's + `:shutdown` travels back up the link while the owner is mid-terminate and + replaces its reason, so a clean `stop(owner, :normal)` exits `:shutdown`. + Letting the link do the work is both simpler and correct. +* A *deliberate* stop (popping a screen) unlinks first, for the same reason — + we are discarding that screen, so its exit must not reflect back. + +### Every owner-to-screen call is protected + +`safe_call/1` wraps all of them, not most. Two were missed in the first cut and +both were fatal: `handle_call(:inspect)` pattern-matched `{:ok, socket}`, so the +one case `safe_call` exists for raised a `MatchError` *inside the owner*; and +`paint_sync` was a raw `GenServer.call`, so a crash in the user's `render/1` — +reached by the everyday "tap a button that pushes a screen" path — exited the +owner. Both defeated the isolation on paths more common than the +`handle_event` crash the tests covered. ### Not a DynamicSupervisor @@ -59,17 +89,58 @@ defeating the isolation on the one path MOB-112's acceptance names explicitly. Every owner-to-screen call goes through `safe_call/1`, which catches the exit and lets the monitor repair the screen. +### A navigation entry carries what a restart needs + +An entry is `%{module:, pid:, params:, ref:}`, not `{module, pid}`. The params +and ref are not bookkeeping — a restart is wrong without them: + +* A screen that mounts on `%{id: id}` cannot come back from `%{}`. The re-mount + raises, the restart fails, and the owner is left holding a dead pid as + `current` — every later event calls a corpse and returns `:ok`, so the app + freezes silently. +* A screen parked under an inactive stack must keep *that stack's* render ref. + Restarting it with the active ref means its next repaint commits over the + foreground tab, undoing the drop-inactive mechanism MOB-110 built. + ### A restart re-mounts, and says so A restarted screen runs `mount/3` again and loses its assigns; persisted -screens get their dumped state back through `load_state/2`. This is logged at -error, because it is visible to the user — a form clears, a list resets — and -silently losing state is worse than saying why. +screens get their dumped state back through `load_state/2`. Logged at error, +because it is visible to the user — a form clears, a list resets — and silently +losing state is worse than saying why. + +When the re-mount itself fails, the owner does not leave the corpse in place: +a background screen is dropped from its stack, and a current screen pops to +whatever is beneath it. With nothing beneath, it logs that the app has no live +screen rather than pretending otherwise. + +Background screens are re-mounted eagerly rather than lazily. A crash is rare, +and keeping every nav entry a live pid means popping or switching back never +has to handle a corpse. + +### A no-op navigation still paints + +The screen deliberately does not paint when it produced a nav action, so every +branch of `apply_nav_action/3` that changes nothing has to paint instead — +`:noop` switch_tab, `pop` at root, `pop_to` not found, and each `start_screen` +failure. Without that, `socket |> assign(:x, v) |> switch_tab(:home)` while +already on `:home` updates the assigns and never renders them. Re-tapping the +active tab is the everyday case, not an exotic one. + +### Mutate navigation only after the mount succeeds + +`switch_tab`'s `mount_root` starts the screen before touching `nav` or the +sender's active ref. Doing it the other way round leaves the sender addressing +a stack whose screen never started, so every frame the live screen produces is +dropped — a silent freeze — while nav holds the same pid in two places. + +### Owner-to-screen calls have no deadline -Background screens (in the active stack's history, or parked under another -stack) are re-mounted eagerly rather than lazily. A crash is rare, and keeping -every nav entry a live pid means popping or switching back never has to handle -a corpse. +`dispatch/3` and `render_sync/2` pass `:infinity`. The screen returns its nav +action *in the reply*, having already cleared it from its socket, so a timeout +does not fail the event — it silently discards the navigation the user asked +for. A slow `handle_event` is a slow app; it is not a lost push. A deliberate +stop does have a bound, so one wedged screen cannot block teardown forever. ### Popping stops the screen; the ones below stay resident @@ -113,5 +184,9 @@ not yank the stack out from under what the user is looking at. - Anything else addressed to `:mob_screen` — device events, notifications, plugin messages — is forwarded by the owner to the active screen. - An owner-to-screen call returns `nil` rather than raising when the screen is - mid-crash. Callers of `get_socket/1` in that window see `nil`; the monitor - repairs the screen immediately after. + mid-crash. `get_socket/1` and `Mob.Test.assigns/1` document and handle that. +- **`Mob.Test.settle/2` now drains three processes, not two.** `:mob_screen` is + the navigation owner; it forwards to the screen, which builds the tree, which + the sender commits. Draining only the owner proved nothing — every + `tap -> settle -> screenshot` sequence in the agent workflow would have been + newly racy. diff --git a/lib/mob/screen.ex b/lib/mob/screen.ex index 21fc51d6..44969d5f 100644 --- a/lib/mob/screen.ex +++ b/lib/mob/screen.ex @@ -2,15 +2,22 @@ defmodule Mob.Screen do @moduledoc """ Behaviour and GenServer wrapper for a Mob screen. - Each screen runs as a supervised GenServer whose state is a `Mob.Socket`. - Putting one process per screen — instead of one big process for the whole - app — gives you isolation: a buggy `handle_event` crashes its own screen - and the supervisor restarts it without taking down navigation, audio, - background services, or the BEAM itself. Lifecycle callbacks (`mount`, - `render`, `handle_event`, `handle_info`, `terminate`) map directly to the - GenServer lifecycle, so the BEAM's existing concurrency tools (selective - receive, monitors, hot code push) work on screens without any Mob-specific - scaffolding. + Each live screen runs in its own `Mob.Screen.Server` process holding a + `Mob.Socket`. This module is their **owner**: it holds the navigation state, + starts and stops screens, and restarts one that crashes. + + That gives you isolation — a buggy `handle_event` crashes its own screen and + the owner restarts it without taking down navigation, sibling screens, + background services, or the BEAM. The owner is not an OTP `Supervisor`; it + restarts screens itself because only it knows where in the navigation a + crashed screen sat (see + `decisions/2026-08-28-screen-processes-and-supervision.md`). A restarted + screen re-mounts and loses its assigns. + + Lifecycle callbacks (`mount`, `render`, `handle_event`, `handle_info`, + `terminate`) map directly to the GenServer lifecycle, so the BEAM's existing + tools (selective receive, monitors, hot code push) work on screens without + any Mob-specific scaffolding. ## Usage @@ -165,14 +172,22 @@ defmodule Mob.Screen do # registered name, so the native layer's `enif_whereis_pid` lookups (back # gesture, alert actions, launch notifications) are unaffected. # - # Its state mirrors what it held before MOB-112 — `{module, socket, nav, - # render_mode}` — with the socket replaced by the pid of the process that now - # owns it. MOB-113 extracts this role into `Mob.Router`. + # A navigation entry is `%{module:, pid:, params:, ref:}`. The params and ref + # are carried because a restart has to reproduce the screen exactly: a screen + # that mounts on `%{id: id}` cannot come back from `%{}`, and a screen parked + # under an inactive stack must keep that stack's render ref or its next + # repaint would commit over the foreground tab. + # + # MOB-113 extracts this role into `Mob.Router`. use GenServer require Logger + # Stopping a screen must not block the owner forever on one that is wedged in + # a long callback. GenServer.stop/3 otherwise waits :infinity. + @stop_timeout_ms 5_000 + @doc """ Start a screen process linked to the calling process. @@ -194,7 +209,7 @@ defmodule Mob.Screen do Return the navigation history (list of `{module, socket}` pairs, head = most recent). Intended for testing and debugging. """ - @spec get_nav_history(pid()) :: [{module(), Mob.Socket.t()}] + @spec get_nav_history(pid()) :: [{module(), Mob.Socket.t() | nil}] def get_nav_history(pid), do: GenServer.call(pid, :get_nav_history) @doc """ @@ -215,13 +230,15 @@ defmodule Mob.Screen do the event has been processed and the state updated. """ @spec dispatch(pid(), String.t(), map()) :: :ok - def dispatch(pid, event, params), do: GenServer.call(pid, {:event, event, params}) + def dispatch(pid, event, params), do: GenServer.call(pid, {:event, event, params}, :infinity) @doc """ - Return the current socket state of a running screen. + Return the current socket state of a running screen, or `nil` while that + screen is being restarted. + Intended for testing and debugging — not for production app logic. """ - @spec get_socket(pid()) :: socket() + @spec get_socket(pid()) :: socket() | nil def get_socket(pid), do: GenServer.call(pid, :get_socket) @doc """ @@ -237,9 +254,13 @@ defmodule Mob.Screen do @impl GenServer def init({screen_module, params, render_mode, platform}) do - # Registered under :mob_screen so the C layer's mob_handle_back() and the - # launch-notification fallback find the owner. Only in :render mode; - # tests use :no_render and run without a NIF. + # Linked *and* trapping. Linking alone makes the owner die with any screen + # it stops or that crashes; trapping alone orphans every screen when the + # owner dies — and an orphaned persisted screen keeps dumping to + # Mob.ScreenState under the same key as its live replacement. Together the + # owner sees each exit as a message and screens still come down with it. + Process.flag(:trap_exit, true) + if render_mode == :render do Process.register(self(), :mob_screen) # Renders are casts, so a missing sender would blank the screen silently. @@ -251,8 +272,7 @@ defmodule Mob.Screen do # Seed the stacks this app declared. The screen we are about to mount # becomes the active stack's current screen; every other declared stack - # stays unmounted until first visited. With no declaration (or no registry, - # as in tests) this is an empty single-stack state. + # stays unmounted until first visited. nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) ref = Mob.Nav.active_ref(nav) Mob.Sender.set_active(ref) @@ -262,7 +282,7 @@ defmodule Mob.Screen do nav: nav, render_mode: render_mode, platform: platform, - monitors: %{} + screens: %{} } case start_screen(screen_module, params, ref, state) do @@ -288,12 +308,10 @@ defmodule Mob.Screen do @impl GenServer def handle_call({:event, event, params}, _from, state) do - {_module, pid} = state.current - # The whole point of MOB-112: a crash in the user's handle_event must not # come back up this call and take the owner — and with it navigation and - # every sibling screen — down. The :DOWN that follows restarts the screen. - case safe_call(fn -> Mob.Screen.Server.dispatch(pid, event, params) end) do + # every sibling screen — down. The exit that follows restarts the screen. + case safe_call(fn -> Mob.Screen.Server.dispatch(state.current.pid, event, params) end) do {:ok, {:ok, nav_action}} -> {:reply, :ok, apply_nav_action(nav_action, state, :sync)} {:exit, _reason} -> {:reply, :ok, state} end @@ -304,22 +322,15 @@ defmodule Mob.Screen do end def handle_call(:get_socket, _from, state) do - {_module, pid} = state.current - - case safe_call(fn -> Mob.Screen.Server.socket(pid) end) do - {:ok, socket} -> {:reply, socket, state} - {:exit, _reason} -> {:reply, nil, state} - end + {:reply, current_socket(state), state} end def handle_call(:get_screen_pid, _from, state) do - {_module, pid} = state.current - {:reply, pid, state} + {:reply, state.current.pid, state} end def handle_call(:get_current_module, _from, state) do - {module, _pid} = state.current - {:reply, module, state} + {:reply, state.current.module, state} end def handle_call(:get_nav_history, _from, state) do @@ -327,14 +338,14 @@ defmodule Mob.Screen do end def handle_call(:inspect, _from, state) do - {module, pid} = state.current - {:ok, socket} = safe_call(fn -> Mob.Screen.Server.socket(pid) end) + module = state.current.module + socket = current_socket(state) info = %{ screen: module, - assigns: socket.assigns, - nav_history: Enum.map(Mob.Nav.history(state.nav), fn {mod, _} -> mod end), - tree: module.render(socket.assigns) + assigns: socket && socket.assigns, + nav_history: Enum.map(Mob.Nav.history(state.nav), & &1.module), + tree: socket && module.render(socket.assigns) } {:reply, info, state} @@ -344,7 +355,7 @@ defmodule Mob.Screen do def handle_cast(:__mob_hot_reload__, state) do # A broadcast, not one cast: every live screen has to repaint with the # newly loaded code, not just the one on screen. - Enum.each(all_screen_pids(state), &Mob.Screen.Server.hot_reload/1) + Enum.each(all_entries(state), &Mob.Screen.Server.hot_reload(&1.pid)) {:noreply, state} end @@ -353,20 +364,21 @@ defmodule Mob.Screen do # navigation — a background screen's timer must not yank the stack out from # under whatever the user is looking at. def handle_info({:nav_action, action, from}, state) do - {_module, current_pid} = state.current - - if from == current_pid do + if from == state.current.pid do {:noreply, apply_nav_action(action, state, :async)} else {:noreply, state} end end - # A screen crashed. This is the isolation MOB-112 exists to deliver: the - # owner is still here, navigation is intact, and the screen comes back. - def handle_info({:DOWN, _monitor_ref, :process, pid, reason}, state) do - state = %{state | monitors: Map.delete(state.monitors, pid)} - {:noreply, restart_screen(pid, reason, state)} + # A screen exited. Trapping turns this into a message rather than the owner's + # death — the isolation MOB-112 exists to deliver. A pid we have already + # dropped from `screens` was stopped deliberately, so its exit is expected. + def handle_info({:EXIT, pid, reason}, state) do + case Map.pop(state.screens, pid) do + {nil, _screens} -> {:noreply, state} + {entry, screens} -> {:noreply, restart_screen(entry, reason, %{state | screens: screens})} + end end # A notification that launched the app from a killed state. @@ -401,24 +413,27 @@ defmodule Mob.Screen do # Anything else addressed to :mob_screen — device events, notifications, # plugin messages — belongs to the screen the user is looking at. def handle_info(message, state) do - {_module, pid} = state.current - send(pid, message) + send(state.current.pid, message) {:noreply, state} end @impl GenServer - def terminate(_reason, state) do - # Screens are linked, so they come down with us; stopping them explicitly - # is what gives each one its terminate/2 and a final state dump. - Enum.each(all_screen_pids(state), fn pid -> - if Process.alive?(pid), do: GenServer.stop(pid, :shutdown) - end) + def terminate(_reason, _state) do + # Deliberately does NOT stop screens. They are linked and trap exits, so + # each shuts down gracefully on its own — gen_server turns the parent's EXIT + # into a terminate/2 call, which runs the user's terminate/2 and the final + # state dump. + # + # Calling GenServer.stop/3 here instead corrupts the owner's own exit: the + # screen's :shutdown travels back while the owner is mid-terminate and + # replaces its reason, so a clean stop(owner, :normal) exits :shutdown. + :ok end # ── Screen lifecycle ────────────────────────────────────────────────────── # Calling into a screen that is mid-crash must not propagate into the owner. - # The monitor that follows is what actually repairs the screen. + # The exit signal that follows is what actually repairs the screen. defp safe_call(fun) do {:ok, fun.()} catch @@ -435,19 +450,17 @@ defmodule Mob.Screen do platform: state.platform ] - case Mob.Screen.Server.start(opts) do + case Mob.Screen.Server.start_link(opts) do {:ok, pid} -> - monitor_ref = Process.monitor(pid) - {:ok, {module, pid}, %{state | monitors: Map.put(state.monitors, pid, monitor_ref)}} + entry = %{module: module, pid: pid, params: params, ref: ref} + {:ok, entry, %{state | screens: Map.put(state.screens, pid, entry)}} {:error, reason} -> {:error, reason} end end - defp all_screen_pids(state) do - current = if state.current, do: [elem(state.current, 1)], else: [] - + defp all_entries(state) do parked = state.nav |> Map.get(:parked, %{}) @@ -455,51 +468,51 @@ defmodule Mob.Screen do [current | history] end) - (current ++ - Enum.map(Mob.Nav.history(state.nav), &elem(&1, 1)) ++ Enum.map(parked, &elem(&1, 1))) - |> Enum.uniq() + ([state.current] ++ Mob.Nav.history(state.nav) ++ parked) + |> Enum.reject(&is_nil/1) + |> Enum.uniq_by(& &1.pid) end - # A crashed screen is re-mounted in place. It loses its assigns — a restart - # runs mount/3 again — which is the documented consequence of the isolation. - defp restart_screen(dead_pid, reason, state) do - ref = Mob.Nav.active_ref(state.nav) + defp current_socket(state) do + case safe_call(fn -> Mob.Screen.Server.socket(state.current.pid) end) do + {:ok, socket} -> socket + {:exit, _reason} -> nil + end + end - case state.current do - {module, ^dead_pid} -> - log_restart(module, reason) + defp entry_with_socket(entry) do + case safe_call(fn -> Mob.Screen.Server.socket(entry.pid) end) do + {:ok, socket} -> {entry.module, socket} + {:exit, _reason} -> {entry.module, nil} + end + end - case start_screen(module, %{}, ref, state) do - {:ok, entry, state} -> - state = %{state | current: entry} - paint(entry, :none, state) - state + # A crashed screen is re-mounted in place, with the params and stack ref it + # was created with. It loses its assigns — a restart runs mount/3 again — + # which is the documented consequence of the isolation. + defp restart_screen(%{pid: dead_pid} = entry, reason, state) do + log_restart(entry.module, reason) - {:error, _reason} -> - state - end + case start_screen(entry.module, entry.params, entry.ref, state) do + {:ok, new_entry, state} -> + state = substitute(state, dead_pid, new_entry) + if state.current.pid == new_entry.pid, do: paint(new_entry, :none, state) + state - _ -> - replace_background_screen(dead_pid, reason, state) + {:error, mount_reason} -> + # Re-mounting failed, so the screen cannot come back. Leaving the dead + # entry in place would freeze the app silently — every later event + # would call a corpse and return :ok. Pop to whatever is underneath if + # there is anything; otherwise say so loudly rather than pretend. + log_restart_failure(entry.module, mount_reason) + recover_from_failed_restart(entry, state) end end - # A background screen (in the active stack's history, or parked under another - # stack) is re-mounted eagerly rather than lazily. A crash is rare, and - # keeping every entry a live pid means popping or switching back never has to - # deal with a corpse. - defp replace_background_screen(dead_pid, reason, state) do + defp substitute(state, dead_pid, new_entry) do replace = fn - {module, ^dead_pid} = entry -> - log_restart(module, reason) - - case start_screen(module, %{}, Mob.Nav.active_ref(state.nav), state) do - {:ok, new_entry, _state} -> new_entry - {:error, _} -> entry - end - - entry -> - entry + %{pid: ^dead_pid} -> new_entry + other -> other end nav = @@ -507,16 +520,43 @@ defmodule Mob.Screen do |> Mob.Nav.put_history(Enum.map(Mob.Nav.history(state.nav), replace)) |> Mob.Nav.map_parked(replace) - # Re-monitor whatever came back. Cheap, and it keeps `monitors` honest - # without threading state through the replace function. - monitors = - nav - |> then(fn n -> %{state | nav: n} end) - |> all_screen_pids() - |> Enum.reject(&Map.has_key?(state.monitors, &1)) - |> Map.new(&{&1, Process.monitor(&1)}) + current = if state.current.pid == dead_pid, do: new_entry, else: state.current + %{state | nav: nav, current: current} + end + + defp recover_from_failed_restart(%{pid: dead_pid} = entry, state) do + cond do + state.current.pid != dead_pid -> + # A background screen. Drop it from the stack it sat in rather than + # leave a corpse for a later pop or tab switch to restore. + drop_entry(state, dead_pid) + + Mob.Nav.history(state.nav) != [] -> + state = drop_entry(state, dead_pid) + [previous | rest] = Mob.Nav.history(state.nav) + state = %{state | nav: Mob.Nav.put_history(state.nav, rest), current: previous} + paint(previous, :pop, state) + state - %{state | nav: nav, monitors: Map.merge(state.monitors, monitors)} + true -> + Logger.error( + "[mob] #{inspect(entry.module)} could not be restarted and there is no screen " <> + "beneath it. The app has no live screen." + ) + + state + end + end + + defp drop_entry(state, dead_pid) do + keep = fn %{pid: pid} -> pid != dead_pid end + + nav = + state.nav + |> Mob.Nav.put_history(Enum.filter(Mob.Nav.history(state.nav), keep)) + |> Mob.Nav.map_parked(& &1) + + %{state | nav: nav} end defp log_restart(module, reason) do @@ -526,31 +566,48 @@ defmodule Mob.Screen do ) end - defp entry_with_socket({module, pid}) do - case safe_call(fn -> Mob.Screen.Server.socket(pid) end) do - {:ok, socket} -> {module, socket} - {:exit, _reason} -> {module, nil} - end + defp log_restart_failure(module, reason) do + Logger.error( + "[mob] screen #{inspect(module)} could not be restarted — mount/3 failed: " <> + "#{inspect(reason)}" + ) end - defp paint(_entry, _transition, %{render_mode: :no_render}), do: :ok - defp paint({_module, pid}, transition, _state), do: Mob.Screen.Server.render(pid, transition) + defp paint(entry, transition, state), do: do_paint(entry, transition, state, :async) - defp paint_sync(_entry, _transition, %{render_mode: :no_render}), do: :ok + defp do_paint(_entry, _transition, %{render_mode: :no_render}, _mode), do: :ok - defp paint_sync({_module, pid}, transition, _state), - do: Mob.Screen.Server.render_sync(pid, transition) + defp do_paint(entry, transition, _state, :sync) do + # Unprotected, this is the other way a screen crash killed the owner: the + # user's render/1 runs inside the screen, and a raise there exits this call. + case safe_call(fn -> Mob.Screen.Server.render_sync(entry.pid, transition) end) do + {:ok, _} -> :ok + {:exit, _reason} -> :ok + end + end - defp do_paint(entry, transition, state, :sync), do: paint_sync(entry, transition, state) - defp do_paint(entry, transition, state, :async), do: paint(entry, transition, state) + defp do_paint(entry, transition, _state, :async), + do: Mob.Screen.Server.render(entry.pid, transition) - # Demonitor before stopping, or the shutdown we asked for comes back as a - # :DOWN and the screen gets "restarted" immediately after being discarded. - defp stop_screen({_module, pid}, state) do - {monitor_ref, monitors} = Map.pop(state.monitors, pid) - if monitor_ref, do: Process.demonitor(monitor_ref, [:flush]) - if Process.alive?(pid), do: GenServer.stop(pid, :shutdown) - %{state | monitors: monitors} + # Drop the entry from tracking BEFORE stopping, so the exit we asked for is + # recognised as deliberate rather than restarted as a crash. + defp stop_screen(%{pid: pid}, state) do + state = %{state | screens: Map.delete(state.screens, pid)} + stop_process(pid) + state + end + + defp stop_process(pid) do + if Process.alive?(pid) do + # Unlink first. We are discarding this screen deliberately, so its + # :shutdown exit must not travel back up the link — during the owner's own + # terminate/2 that signal overrides the owner's exit reason, which turns a + # clean GenServer.stop(owner, :normal) into an exit with :shutdown. + Process.unlink(pid) + GenServer.stop(pid, :shutdown, @stop_timeout_ms) + end + catch + :exit, _reason -> :ok end # ── Navigation ──────────────────────────────────────────────────────────── @@ -560,8 +617,9 @@ defmodule Mob.Screen do defp apply_nav_action({:push, dest, params}, state, mode) do {new_module, route_params} = resolve_destination(dest) ref = Mob.Nav.active_ref(state.nav) + mount_params = Map.merge(route_params, params) - case start_screen(new_module, Map.merge(route_params, params), ref, state) do + case start_screen(new_module, mount_params, ref, state) do {:ok, entry, state} -> nav = Mob.Nav.put_history(state.nav, [state.current | Mob.Nav.history(state.nav)]) state = %{state | nav: nav, current: entry} @@ -569,7 +627,7 @@ defmodule Mob.Screen do state {:error, _reason} -> - state + repaint_current(state, mode) end end @@ -585,21 +643,24 @@ defmodule Mob.Screen do state [] -> - state + repaint_current(state, mode) end end defp apply_nav_action({:pop_to_root}, state, mode) do case Enum.reverse(Mob.Nav.history(state.nav)) do [root | _] -> - discarded = [state.current | Enum.reject(Mob.Nav.history(state.nav), &(&1 == root))] + discarded = [ + state.current | Enum.reject(Mob.Nav.history(state.nav), &(&1.pid == root.pid)) + ] + state = Enum.reduce(discarded, state, &stop_screen/2) state = %{state | nav: Mob.Nav.put_history(state.nav, []), current: root} do_paint(root, :pop, state, mode) state [] -> - state + repaint_current(state, mode) end end @@ -609,22 +670,23 @@ defmodule Mob.Screen do case pop_to_module(history, target) do {:found, previous, rest} -> - discarded = [state.current | Enum.take_while(history, &(&1 != previous))] + discarded = [state.current | Enum.take_while(history, &(&1.pid != previous.pid))] state = Enum.reduce(discarded, state, &stop_screen/2) state = %{state | nav: Mob.Nav.put_history(state.nav, rest), current: previous} do_paint(previous, :pop, state, mode) state :not_found -> - state + repaint_current(state, mode) end end defp apply_nav_action({:reset, dest, params}, state, mode) do {new_module, route_params} = resolve_destination(dest) ref = Mob.Nav.active_ref(state.nav) + mount_params = Map.merge(route_params, params) - case start_screen(new_module, Map.merge(route_params, params), ref, state) do + case start_screen(new_module, mount_params, ref, state) do {:ok, entry, state} -> discarded = [state.current | Mob.Nav.history(state.nav)] state = Enum.reduce(discarded, state, &stop_screen/2) @@ -633,7 +695,7 @@ defmodule Mob.Screen do state {:error, _reason} -> - state + repaint_current(state, mode) end end @@ -648,25 +710,37 @@ defmodule Mob.Screen do state {:mount_root, nav, root_module} -> + # Start first, mutate after. Switching nav and the sender's active ref + # before the mount could fail leaves the sender addressing a stack whose + # screen never started, and every frame the live screen produces is then + # dropped — a silent freeze. ref = Mob.Nav.active_ref(nav) - Mob.Sender.set_active(ref) - state = %{state | nav: nav} case start_screen(root_module, %{}, ref, state) do {:ok, entry, state} -> - state = %{state | current: entry} + Mob.Sender.set_active(ref) + state = %{state | nav: nav, current: entry} do_paint(entry, :none, state, mode) state {:error, _reason} -> - state + repaint_current(state, mode) end :noop -> - state + repaint_current(state, mode) end end + # A nav action that changed nothing still has to paint. The screen deliberately + # does not paint when it produced an action, so without this the assigns it set + # in the same callback would never reach the screen — re-tapping the active tab + # being the everyday case. + defp repaint_current(state, mode) do + do_paint(state.current, :none, state, mode) + state + end + defp resolve_module(dest) when is_atom(dest) do {module, _route_params} = resolve_destination(dest) module @@ -697,7 +771,7 @@ defmodule Mob.Screen do defp pop_to_module([], _target), do: :not_found - defp pop_to_module([{module, _pid} = entry | rest], target) do + defp pop_to_module([%{module: module} = entry | rest], target) do if module == target do {:found, entry, rest} else diff --git a/lib/mob/screen/server.ex b/lib/mob/screen/server.ex index b14aa911..b872766c 100644 --- a/lib/mob/screen/server.ex +++ b/lib/mob/screen/server.ex @@ -49,29 +49,30 @@ defmodule Mob.Screen.Server do defstruct [:module, :socket, :render_mode, :ref, :owner] @doc """ - Start a screen process. + Start a screen linked to the calling process. - `:owner` receives nav actions and monitors this process. `:ref` is the - navigation stack this screen belongs to, used to address its renders at - `Mob.Sender`. + `:owner` receives nav actions and the exit signal. `:ref` is the navigation + stack this screen belongs to, used to address its renders at `Mob.Sender`. + + `Mob.Screen` links *and* traps exits. Linking alone would make the owner die + with any screen it stopped or that crashed; trapping alone would leave every + screen orphaned when the owner died — and an orphaned persisted screen keeps + dumping to `Mob.ScreenState` under the same key as its live replacement. + Together the owner observes each exit as a message without sharing its fate, + and screens still come down with it. """ @spec start_link(keyword()) :: GenServer.on_start() def start_link(opts), do: GenServer.start_link(__MODULE__, opts) - @doc """ - Start a screen process **unlinked**. - - This is what `Mob.Screen` uses. Linking would defeat the point: the owner - would die with any screen it stopped or that crashed, taking navigation and - every sibling screen with it — exactly the coupling MOB-112 removes. The - owner monitors instead, so it observes the exit without sharing its fate. - """ - @spec start(keyword()) :: GenServer.on_start() - def start(opts), do: GenServer.start(__MODULE__, opts) - @doc "Run a user event, returning any navigation action it produced." @spec dispatch(pid(), String.t(), map()) :: {:ok, term() | nil} - def dispatch(pid, event, params), do: GenServer.call(pid, {:event, event, params}) + def dispatch(pid, event, params) do + # No deadline. The screen returns its nav action in the reply, having + # already cleared it from its socket, so a timeout here does not fail the + # event — it silently discards the navigation the user asked for. A slow + # handle_event is a slow app; it is not a lost push. + GenServer.call(pid, {:event, event, params}, :infinity) + end @doc "This screen's current socket." @spec socket(pid()) :: Mob.Socket.t() @@ -83,11 +84,11 @@ defmodule Mob.Screen.Server do @doc "Paint and block until the frame has been committed." @spec render_sync(pid(), atom()) :: :ok - def render_sync(pid, transition \\ :none), do: GenServer.call(pid, {:render_sync, transition}) - - @doc "Tell this screen which stack it now belongs to." - @spec set_ref(pid(), render_ref()) :: :ok - def set_ref(pid, ref), do: GenServer.cast(pid, {:set_ref, ref}) + def render_sync(pid, transition \\ :none) do + # Matches Mob.Sender.sync(:infinity) one hop down: rendering was never + # time-bounded, and bounding it here would kill the screen on a slow frame. + GenServer.call(pid, {:render_sync, transition}, :infinity) + end @doc "Repaint with the screen module's newly loaded code." @spec hot_reload(pid()) :: :ok @@ -97,6 +98,12 @@ defmodule Mob.Screen.Server do @impl GenServer def init(opts) do + # Trapping so this screen shuts down gracefully when its owner exits: + # gen_server turns the parent's EXIT into a terminate/2 call, which is what + # runs the user's terminate/2 and the final Mob.ScreenState dump. Without + # it a screen is killed by the link and neither happens. + Process.flag(:trap_exit, true) + module = Keyword.fetch!(opts, :module) render_mode = Keyword.get(opts, :render_mode, :no_render) platform = Keyword.get(opts, :platform, :android) @@ -145,8 +152,6 @@ defmodule Mob.Screen.Server do {:noreply, %{state | socket: paint(state, transition)}} end - def handle_cast({:set_ref, ref}, state), do: {:noreply, %{state | ref: ref}} - def handle_cast(:__mob_hot_reload__, state) do {:noreply, %{state | socket: paint(state, :none)}} end diff --git a/lib/mob/test.ex b/lib/mob/test.ex index bde72934..eceac67f 100644 --- a/lib/mob/test.ex +++ b/lib/mob/test.ex @@ -67,8 +67,9 @@ defmodule Mob.Test do Mob.Test.assigns(node) `:sys.get_state/1` on `:mob_screen` is no longer sufficient on its own: since - MOB-110 the screen hands its tree to `Mob.Sender` and returns, so a drained - screen mailbox does not mean the frame is on screen. That only matters for the + MOB-110 the tree is handed to `Mob.Sender` and committed asynchronously, and + since MOB-112 `:mob_screen` is the navigation owner rather than the screen + itself. That only matters for the functions that read the *native* side — `view_tree/1`, `screenshot/2`, `tap_id/2`, `element_frames/2`. `tree/1` and `assigns/1` re-render in-process and are unaffected. @@ -176,9 +177,18 @@ defmodule Mob.Test do @spec screen(node()) :: module() def screen(node), do: rpc(node, :get_current_module) - @doc "Return the current screen's assigns map." - @spec assigns(node()) :: map() - def assigns(node), do: rpc(node, :get_socket).assigns + @doc """ + Return the current screen's assigns map, or `nil` while that screen is being + restarted after a crash (MOB-112 — the socket lives in the screen's own + process, which is briefly absent). + """ + @spec assigns(node()) :: map() | nil + def assigns(node) do + case rpc(node, :get_socket) do + nil -> nil + socket -> socket.assigns + end + end @doc """ Return a map with `:screen`, `:assigns`, `:nav_history`, and `:tree` @@ -228,9 +238,10 @@ defmodule Mob.Test do Block until the app has finished processing and the current frame is on screen. - Drains the screen process's mailbox, then waits for `Mob.Sender` to commit. - Both halves are needed: the screen builds the tree and the sender commits it, - so a drained screen mailbox alone does not mean the frame has been rendered. + Drains the navigation owner, then the screen process, then waits for + `Mob.Sender` to commit. All three are needed: the owner forwards the event, + the screen builds the tree, and the sender commits it — so a drained owner + mailbox alone does not mean the frame has been rendered. Use after any fire-and-forget call (`tap/2`, `back/1`, `send_message/2`) before reading the native side with `view_tree/1`, `screenshot/2`, `tap_id/2` @@ -242,7 +253,17 @@ defmodule Mob.Test do """ @spec settle(node(), timeout()) :: :ok def settle(node, timeout \\ 5000) do + # Three hops, not two. Since MOB-112 the process registered as :mob_screen + # is the navigation *owner*; it forwards events to the screen, which builds + # the tree, which the sender commits. Draining only the owner proves + # nothing about the other two. :rpc.call(node, :sys, :get_state, [:mob_screen]) + + case :rpc.call(node, Mob.Screen, :get_screen_pid, [:mob_screen]) do + pid when is_pid(pid) -> :rpc.call(node, :sys, :get_state, [pid]) + _ -> :ok + end + :rpc.call(node, Mob.Sender, :sync, [timeout]) :ok end diff --git a/test/mob/nav/multi_stack_test.exs b/test/mob/nav/multi_stack_test.exs index c8cf614a..d30ba4c8 100644 --- a/test/mob/nav/multi_stack_test.exs +++ b/test/mob/nav/multi_stack_test.exs @@ -103,14 +103,23 @@ defmodule Mob.Nav.MultiStackTest do end {:ok, pid} = Mob.Nav.Registry.start_link(TabApp) - on_exit(fn -> if Process.alive?(pid), do: GenServer.stop(pid) end) + on_exit(fn -> stop_safely(pid) end) {:ok, screen} = Mob.Screen.start_link(HomeScreen, %{}) - on_exit(fn -> if Process.alive?(screen), do: GenServer.stop(screen) end) + on_exit(fn -> stop_safely(screen) end) %{screen: screen} end + # `if Process.alive?, do: GenServer.stop` races: the process can exit between + # the check and the stop, and the :noproc exit then fails the test from inside + # the on_exit runner. Screens and their owner die with the test process. + defp stop_safely(pid) do + GenServer.stop(pid) + catch + :exit, _ -> :ok + end + describe "switching stacks" do test "first switch mounts the target stack's declared root", %{screen: screen} do Mob.Screen.dispatch(screen, "to_settings", %{}) diff --git a/test/mob/screen/isolation_test.exs b/test/mob/screen/isolation_test.exs index d6fb1b40..a47ea20b 100644 --- a/test/mob/screen/isolation_test.exs +++ b/test/mob/screen/isolation_test.exs @@ -50,7 +50,7 @@ defmodule Mob.Screen.IsolationTest do end defp history_pids(owner) do - owner |> :sys.get_state() |> Map.fetch!(:nav) |> Mob.Nav.history() |> Enum.map(&elem(&1, 1)) + owner |> :sys.get_state() |> Map.fetch!(:nav) |> Mob.Nav.history() |> Enum.map(& &1.pid) end setup do @@ -60,14 +60,23 @@ defmodule Mob.Screen.IsolationTest do end {:ok, registry} = Mob.Nav.Registry.start_link(DemoApp) - on_exit(fn -> if Process.alive?(registry), do: GenServer.stop(registry) end) + on_exit(fn -> stop_safely(registry) end) {:ok, owner} = Mob.Screen.start_link(HomeScreen, %{}) - on_exit(fn -> if Process.alive?(owner), do: GenServer.stop(owner) end) + on_exit(fn -> stop_safely(owner) end) %{owner: owner} end + # `if Process.alive?, do: GenServer.stop` races: the process can exit between + # the check and the stop, and the :noproc exit then fails the test from inside + # the on_exit runner. Screens and their owner die with the test process. + defp stop_safely(pid) do + GenServer.stop(pid) + catch + :exit, _ -> :ok + end + describe "one process per screen" do test "the owner and the screen are different processes", %{owner: owner} do assert Mob.Screen.get_screen_pid(owner) != owner diff --git a/test/mob/screen/restart_test.exs b/test/mob/screen/restart_test.exs new file mode 100644 index 00000000..8d82bf9b --- /dev/null +++ b/test/mob/screen/restart_test.exs @@ -0,0 +1,211 @@ +defmodule Mob.Screen.RestartTest do + @moduledoc """ + Restarting a screen that is *not* the one on screen. + + The first cut of MOB-112 restarted every screen with `%{}` params and the + *active* stack's render ref. Both are wrong for a background screen: a screen + that mounts on `%{id: id}` cannot come back from `%{}`, and a parked screen + tagged with the active ref paints over the foreground tab the next time it + re-renders. + """ + use ExUnit.Case, async: false + + import ExUnit.CaptureLog + + defmodule HomeScreen do + use Mob.Screen + + @detail Mob.Screen.RestartTest.DetailScreen + + def mount(_params, _session, socket), do: {:ok, Mob.Socket.assign(socket, :where, :home)} + def render(assigns), do: %{type: :text, props: %{text: "#{assigns.where}"}, children: []} + + def handle_event("push", _, socket), + do: {:noreply, Mob.Socket.push_screen(socket, @detail, %{id: 42})} + + def handle_event("to_settings", _, socket), + do: {:noreply, Mob.Socket.switch_tab(socket, :settings)} + + def handle_event("to_home", _, socket), + do: {:noreply, Mob.Socket.switch_tab(socket, :home)} + end + + defmodule DetailScreen do + use Mob.Screen + + # Mounts on a required param. A restart that forgets it cannot come back. + def mount(%{id: id}, _session, socket), do: {:ok, Mob.Socket.assign(socket, :id, id)} + def render(assigns), do: %{type: :text, props: %{text: "detail #{assigns.id}"}, children: []} + + def handle_event("to_settings", _, socket), + do: {:noreply, Mob.Socket.switch_tab(socket, :settings)} + end + + defmodule SettingsScreen do + use Mob.Screen + + def mount(_params, _session, socket), do: {:ok, Mob.Socket.assign(socket, :where, :settings)} + def render(assigns), do: %{type: :text, props: %{text: "#{assigns.where}"}, children: []} + + def handle_event("to_home", _, socket), + do: {:noreply, Mob.Socket.switch_tab(socket, :home)} + end + + defmodule TabApp do + @behaviour Mob.App + import Mob.App + + @home Mob.Screen.RestartTest.HomeScreen + @settings Mob.Screen.RestartTest.SettingsScreen + + def navigation(_) do + tab_bar([stack(:home, root: @home), stack(:settings, root: @settings)]) + end + end + + defp stop_safely(pid) do + GenServer.stop(pid) + catch + :exit, _ -> :ok + end + + defp owner_state(owner), do: :sys.get_state(owner) + defp history(owner), do: owner |> owner_state() |> Map.fetch!(:nav) |> Mob.Nav.history() + defp parked(owner), do: owner |> owner_state() |> Map.fetch!(:nav) |> Map.fetch!(:parked) + + defp kill_and_settle(owner, pid) do + capture_log(fn -> + Process.exit(pid, :kill) + # Let the owner process the EXIT and finish the restart. + :sys.get_state(owner) + :sys.get_state(owner) + end) + end + + setup do + case Process.whereis(Mob.Nav.Registry) do + nil -> :ok + pid -> GenServer.stop(pid) + end + + {:ok, registry} = Mob.Nav.Registry.start_link(TabApp) + on_exit(fn -> stop_safely(registry) end) + + {:ok, owner} = Mob.Screen.start_link(HomeScreen, %{}) + on_exit(fn -> stop_safely(owner) end) + + %{owner: owner} + end + + describe "a screen in the active stack's history" do + test "is restarted rather than left as a corpse", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + [home] = history(owner) + + kill_and_settle(owner, home.pid) + + [restarted] = history(owner) + assert restarted.pid != home.pid + assert Process.alive?(restarted.pid) + assert restarted.module == HomeScreen + end + + test "the screen on top is untouched", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + detail = Mob.Screen.get_screen_pid(owner) + [home] = history(owner) + + kill_and_settle(owner, home.pid) + + assert Mob.Screen.get_screen_pid(owner) == detail + assert Mob.Screen.get_current_module(owner) == DetailScreen + end + + test "popping back reaches the restarted screen, not the dead one", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + [home] = history(owner) + kill_and_settle(owner, home.pid) + + :ok = GenServer.call(owner, {:navigate, {:pop}}) + + assert Mob.Screen.get_current_module(owner) == HomeScreen + assert Mob.Screen.get_socket(owner).assigns.where == :home + assert Mob.Screen.get_nav_history(owner) == [] + end + end + + describe "restart reproduces the screen" do + test "a screen that mounts on params comes back with them", %{owner: owner} do + # Restarting with %{} would raise FunctionClauseError in mount/3 and the + # screen would never return. + Mob.Screen.dispatch(owner, "push", %{}) + detail = Mob.Screen.get_screen_pid(owner) + assert Mob.Screen.get_socket(owner).assigns.id == 42 + + kill_and_settle(owner, detail) + + assert Mob.Screen.get_screen_pid(owner) != detail + assert Mob.Screen.get_current_module(owner) == DetailScreen + assert Mob.Screen.get_socket(owner).assigns.id == 42 + end + end + + describe "a parked screen under an inactive stack" do + test "stays alive across a tab switch", %{owner: owner} do + home = Mob.Screen.get_screen_pid(owner) + Mob.Screen.dispatch(owner, "to_settings", %{}) + + assert Process.alive?(home) + assert Mob.Screen.get_current_module(owner) == SettingsScreen + end + + test "is restarted with its OWN stack's ref, not the active one", %{owner: owner} do + home = Mob.Screen.get_screen_pid(owner) + assert :sys.get_state(home).ref == :home + + Mob.Screen.dispatch(owner, "to_settings", %{}) + kill_and_settle(owner, home) + + restarted = parked(owner)[:home].current + assert restarted.pid != home + assert restarted.ref == :home, "a parked screen tagged :settings would paint over the tab" + assert :sys.get_state(restarted.pid).ref == :home + end + + test "switching back reaches the restarted screen", %{owner: owner} do + home = Mob.Screen.get_screen_pid(owner) + Mob.Screen.dispatch(owner, "to_settings", %{}) + kill_and_settle(owner, home) + + Mob.Screen.dispatch(owner, "to_home", %{}) + + assert Mob.Screen.get_current_module(owner) == HomeScreen + assert Mob.Screen.get_screen_pid(owner) != home + assert Process.alive?(Mob.Screen.get_screen_pid(owner)) + end + end + + describe "bookkeeping" do + test "a restarted screen is monitored exactly once", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + [home] = history(owner) + kill_and_settle(owner, home.pid) + + [restarted] = history(owner) + links = owner |> Process.info(:links) |> elem(1) + assert Enum.count(links, &(&1 == restarted.pid)) == 1 + end + + test "a deliberately popped screen is not restarted", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + detail = Mob.Screen.get_screen_pid(owner) + + :ok = GenServer.call(owner, {:navigate, {:pop}}) + :sys.get_state(owner) + + refute Process.alive?(detail) + assert Mob.Screen.get_current_module(owner) == HomeScreen + assert Mob.Screen.get_nav_history(owner) == [] + end + end +end diff --git a/test/mob/screen_sender_wiring_test.exs b/test/mob/screen_sender_wiring_test.exs index c8f3ffcf..0ae0177e 100644 --- a/test/mob/screen_sender_wiring_test.exs +++ b/test/mob/screen_sender_wiring_test.exs @@ -72,15 +72,24 @@ defmodule Mob.ScreenSenderWiringTest do {:ok, registry} = Mob.Nav.Registry.start_link(TabApp) on_exit(fn -> - for pid <- [sender, registry], Process.alive?(pid), do: GenServer.stop(pid) + for pid <- [sender, registry], do: stop_safely(pid) end) {:ok, screen} = Mob.Screen.start_link(HomeScreen, %{}) - on_exit(fn -> if Process.alive?(screen), do: GenServer.stop(screen) end) + on_exit(fn -> stop_safely(screen) end) %{screen: screen} end + # `if Process.alive?, do: GenServer.stop` races: the process can exit between + # the check and the stop, and the :noproc exit then fails the test from inside + # the on_exit runner. Screens and their owner die with the test process. + defp stop_safely(pid) do + GenServer.stop(pid) + catch + :exit, _ -> :ok + end + test "mounting a screen declares its stack active", %{screen: _} do Sender.sync() assert active() == :home From 3d297fc45824399354e786da82ddc4aa10c219b9 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 28 Aug 2026 22:12:03 -0600 Subject: [PATCH 3/5] MOB-112: per-screen render refs, restart ceiling, and review #2 fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blocking find from review #2 was a regression this change introduced, not a pre-existing gap. Making every screen a live process means the ones *below* the top of the stack now receive messages and repaint — but Mob.Sender was still addressed per navigation stack, and every screen in a stack shared one ref. So a Process.send_after tick, a Task reply, or a PubSub broadcast landing in a screen the user cannot see committed its tree — tap table included — over the screen they can. MOB-107's whole point is that those messages now arrive on the screen's own pid, so this is the ordinary case, not an exotic one. No test could see it: every test runs :no_render, where paint/3 short-circuits. The render ref is now unique per SCREEN, minted at start and preserved across restarts, and Mob.Sender.set_active/1 is called from exactly one place — the new make_current/2, the single point where `current` changes. The sender only ever commits the active ref, so a background repaint is dropped wherever that screen sits. This also subsumes the parked-screen-ref problem review #1 found, and fixes the same bug in the __mob_hot_reload__ broadcast, which repainted every live screen and let the last to arrive win. Mob.Sender's own @type doc anticipated this ("MOB-112 replaces it with a per-screen reference"); the first cut did not. Other confirmed findings: - drop_entry/2 called Mob.Nav.map_parked(& &1) — the identity function — so it dropped nothing from parked. A screen parked under an inactive tab whose re-mount failed stayed in nav as a dead pid, and switching to that tab restored a corpse, freezing it permanently. Mapping cannot express this: dropping a stack's *current* has to collapse the stack. Added Mob.Nav.drop_parked/2, which promotes the history head, or removes the stack entirely so the next switch mounts its root fresh. - Restarts had no ceiling. A screen that mounts cleanly and crashes on every render looped at ~6500 restarts/sec, one log line each. The owner restarts screens itself because it is the only thing that knows where one sat, so it now carries the max-restart-intensity a supervisor would have given: 5 in 10s per screen ref, then it gives up and falls back to the screen beneath. - handle_call(:inspect) still ran the user's render/1 in the OWNER. Review #1 fixed the MatchError above it and left this. A raise there — reached by Mob.Test.tree/1, the debugging path — killed navigation and every screen. The tree is now built in the screen's own process. - A stop that timed out left the screen alive, unlinked and untracked, still dumping to Mob.ScreenState under its replacement's key — verbatim the orphan hazard the ADR uses to justify linking. It is killed outright now. - Mob.Test.settle/2 drained the screen that was on its way OUT: a navigating tap moves owner -> old screen -> owner -> new screen, so settle returned before the incoming screen had rendered and tap -> settle -> screenshot read the stale frame. It drains twice now. - Screens trapping exits silently swallowed linked-task crashes into the user's default handle_info. They are logged before being forwarded. - The ADR claimed the cost of linking was orphaning on Process.exit(owner, :kill). That is wrong — :killed is trappable, so screens still run terminate/2. Replaced with the two real costs. Tests: 7 more (drop_parked, the restart ceiling, owner-safe inspection, and a background screen's ref). Verified as negative controls — reverting to a stack-wide ref fails exactly the test written for it. The clobber fix is proven in two parts that hold without :render mode: the sender drops non-active refs, and background screens have non-active refs. Suite 1231 passed, format and credo --strict clean, 10/10 clean runs across the touched test files. Re-verified on the iOS simulator: cold boot, push (own pid #PID<0.146.0>), pop, zero errors in the BEAM log. Co-Authored-By: Claude Opus 5 (1M context) --- ...-08-28-screen-processes-and-supervision.md | 16 +- lib/mob/nav.ex | 29 ++++ lib/mob/screen.ex | 147 ++++++++++++------ lib/mob/screen/server.ex | 30 ++++ lib/mob/test.ex | 26 +++- test/mob/nav_test.exs | 49 ++++++ test/mob/screen/restart_test.exs | 70 ++++++++- test/mob/screen_sender_wiring_test.exs | 56 +++++-- 8 files changed, 351 insertions(+), 72 deletions(-) diff --git a/decisions/2026-08-28-screen-processes-and-supervision.md b/decisions/2026-08-28-screen-processes-and-supervision.md index 0fdf3dba..3ff6e5d7 100644 --- a/decisions/2026-08-28-screen-processes-and-supervision.md +++ b/decisions/2026-08-28-screen-processes-and-supervision.md @@ -76,10 +76,18 @@ tab, and restoring it means putting the new pid back exactly where the old one was. The owner is the only thing that knows that, so it owns the restart. This is the "deliberate restart strategy" MOB-112 asks for. -The cost: a screen orphans if the owner is killed without running `terminate/2` -(`Process.exit(owner, :kill)`). Acceptable — the owner dying means the app is -going down — but a `DynamicSupervisor` under a real supervision tree would -close it, and mob does not have one yet. +An earlier draft of this file claimed the cost was orphaning on +`Process.exit(owner, :kill)`. That is wrong: `:killed` is a trappable reason for +the *linked screens*, so each still runs `terminate/2` and its final dump. + +The real cost is the restart ceiling a supervisor would have given for free. +The owner carries its own (`@max_restarts` in `@restart_window_ms`, per screen +ref) because without it a screen that mounts cleanly and crashes on every render +loops at roughly 6500 restarts a second, writing a log line each time. The other +gap is a screen wedged in a callback: the owner's bounded `GenServer.stop/3` +kills it outright on timeout rather than leaving it unlinked and untracked but +alive, still dumping to `Mob.ScreenState` under the same key as its +replacement. ### A crash must not come back up a call diff --git a/lib/mob/nav.ex b/lib/mob/nav.ex index 7c09c701..2aec87c3 100644 --- a/lib/mob/nav.ex +++ b/lib/mob/nav.ex @@ -166,6 +166,35 @@ defmodule Mob.Nav do %{nav | parked: parked} end + @doc """ + Remove every parked entry for which `fun` returns true. + + Dropping is not mapping: a stack whose *current* entry goes away has to + collapse. The head of its history is promoted; a stack left with nothing at + all is removed from `parked` entirely, so the next switch to it mounts its + root fresh rather than restoring a screen that is gone. + + `Mob.Screen` uses this when a screen crashes and cannot be re-mounted — + leaving the dead entry in place would freeze that tab permanently, since + switching to it would restore a corpse. + """ + @spec drop_parked(t(), (entry() -> boolean())) :: t() + def drop_parked(%__MODULE__{parked: parked} = nav, fun) when is_function(fun, 1) do + parked = + parked + |> Enum.reduce(%{}, fn {name, %{current: current, history: history}}, acc -> + history = Enum.reject(history, fun) + + cond do + not fun.(current) -> Map.put(acc, name, %{current: current, history: history}) + history == [] -> acc + true -> Map.put(acc, name, %{current: hd(history), history: tl(history)}) + end + end) + + %{nav | parked: parked} + end + @doc """ Switch the active stack to `name`, parking `current_entry` under the stack it belongs to. diff --git a/lib/mob/screen.ex b/lib/mob/screen.ex index 44969d5f..e591c269 100644 --- a/lib/mob/screen.ex +++ b/lib/mob/screen.ex @@ -172,11 +172,19 @@ defmodule Mob.Screen do # registered name, so the native layer's `enif_whereis_pid` lookups (back # gesture, alert actions, launch notifications) are unaffected. # - # A navigation entry is `%{module:, pid:, params:, ref:}`. The params and ref - # are carried because a restart has to reproduce the screen exactly: a screen - # that mounts on `%{id: id}` cannot come back from `%{}`, and a screen parked - # under an inactive stack must keep that stack's render ref or its next - # repaint would commit over the foreground tab. + # A navigation entry is `%{module:, pid:, params:, ref:}`. + # + # `params` is carried because a restart has to reproduce the screen exactly — + # one that mounts on `%{id: id}` cannot come back from `%{}`. + # + # `ref` identifies the screen to `Mob.Sender`, and is unique **per screen**, + # not per stack. That distinction is load-bearing: every screen is a live + # process now, including the ones below the top of the stack, and they repaint + # on any message they receive. Keyed by stack, a timer tick in a screen the + # user cannot see would commit its tree — tap table included — over the screen + # they can. The sender only ever commits the tree whose ref is active, so a + # background repaint is dropped wherever that screen sits. The ref survives a + # restart, because the replacement is the same logical screen. # # MOB-113 extracts this role into `Mob.Router`. @@ -188,6 +196,14 @@ defmodule Mob.Screen do # a long callback. GenServer.stop/3 otherwise waits :infinity. @stop_timeout_ms 5_000 + # A screen that mounts fine but crashes on every render otherwise loops at + # full speed — measured at ~6500 restarts/sec, each writing a log line. An + # OTP supervisor would cap this with max_restart_intensity; the owner restarts + # screens itself (it is the only thing that knows where one sat), so it has to + # carry the ceiling too. + @max_restarts 5 + @restart_window_ms 10_000 + @doc """ Start a screen process linked to the calling process. @@ -247,7 +263,7 @@ defmodule Mob.Screen do Each live screen is its own process since MOB-112; this is how tooling reaches the one that is on screen. """ - @spec get_screen_pid(pid()) :: pid() + @spec get_screen_pid(GenServer.server()) :: pid() def get_screen_pid(pid), do: GenServer.call(pid, :get_screen_pid) # ── GenServer callbacks ─────────────────────────────────────────────────── @@ -274,19 +290,20 @@ defmodule Mob.Screen do # becomes the active stack's current screen; every other declared stack # stays unmounted until first visited. nav = Mob.Nav.from_layout(Mob.Nav.Registry.layout(platform), screen_module) - ref = Mob.Nav.active_ref(nav) - Mob.Sender.set_active(ref) state = %{ current: nil, nav: nav, render_mode: render_mode, platform: platform, - screens: %{} + screens: %{}, + restarts: %{} } - case start_screen(screen_module, params, ref, state) do + case start_screen(screen_module, params, state) do {:ok, entry, state} -> + state = make_current(state, entry) + if render_mode == :render do # A notification that launched the app from a killed state. Sent to # self so it arrives via handle_info after init returns, consistent @@ -299,7 +316,7 @@ defmodule Mob.Screen do paint(entry, :none, state) end - {:ok, %{state | current: entry}} + {:ok, state} {:error, reason} -> {:stop, reason} @@ -338,14 +355,22 @@ defmodule Mob.Screen do end def handle_call(:inspect, _from, state) do - module = state.current.module socket = current_socket(state) + # The tree is built in the screen's own process. Calling render/1 here would + # run user code in the owner, so a raise in it — reached by Mob.Test.tree/1, + # i.e. the debugging path — would kill navigation and every screen. + tree = + case safe_call(fn -> Mob.Screen.Server.tree(state.current.pid) end) do + {:ok, tree} -> tree + {:exit, _reason} -> nil + end + info = %{ - screen: module, + screen: state.current.module, assigns: socket && socket.assigns, nav_history: Enum.map(Mob.Nav.history(state.nav), & &1.module), - tree: socket && module.render(socket.assigns) + tree: tree } {:reply, info, state} @@ -440,6 +465,8 @@ defmodule Mob.Screen do :exit, reason -> {:exit, reason} end + defp start_screen(module, params, state), do: start_screen(module, params, make_ref(), state) + defp start_screen(module, params, ref, state) do opts = [ module: module, @@ -460,6 +487,13 @@ defmodule Mob.Screen do end end + # The single place `current` changes. The sender is told here and nowhere + # else, so only the screen the user is looking at can commit a frame. + defp make_current(state, entry) do + Mob.Sender.set_active(entry.ref) + %{state | current: entry} + end + defp all_entries(state) do parked = state.nav @@ -490,7 +524,32 @@ defmodule Mob.Screen do # A crashed screen is re-mounted in place, with the params and stack ref it # was created with. It loses its assigns — a restart runs mount/3 again — # which is the documented consequence of the isolation. - defp restart_screen(%{pid: dead_pid} = entry, reason, state) do + defp restart_screen(entry, reason, state) do + {allowed?, state} = record_restart(entry.ref, state) + + if allowed? do + do_restart_screen(entry, reason, state) + else + Logger.error( + "[mob] screen #{inspect(entry.module)} crashed #{@max_restarts + 1} times in " <> + "#{@restart_window_ms}ms and is being given up on rather than restarted in a loop. " <> + "Reason: #{inspect(reason)}" + ) + + recover_from_failed_restart(entry, state) + end + end + + # Sliding window per screen ref, so the ceiling follows a logical screen + # across its restarts rather than resetting with each new pid. + defp record_restart(ref, state) do + now = System.monotonic_time(:millisecond) + recent = Map.get(state.restarts, ref, []) |> Enum.filter(&(now - &1 < @restart_window_ms)) + state = %{state | restarts: Map.put(state.restarts, ref, [now | recent])} + {length(recent) < @max_restarts, state} + end + + defp do_restart_screen(%{pid: dead_pid} = entry, reason, state) do log_restart(entry.module, reason) case start_screen(entry.module, entry.params, entry.ref, state) do @@ -534,7 +593,7 @@ defmodule Mob.Screen do Mob.Nav.history(state.nav) != [] -> state = drop_entry(state, dead_pid) [previous | rest] = Mob.Nav.history(state.nav) - state = %{state | nav: Mob.Nav.put_history(state.nav, rest), current: previous} + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) paint(previous, :pop, state) state @@ -549,12 +608,12 @@ defmodule Mob.Screen do end defp drop_entry(state, dead_pid) do - keep = fn %{pid: pid} -> pid != dead_pid end + dead? = fn %{pid: pid} -> pid == dead_pid end nav = state.nav - |> Mob.Nav.put_history(Enum.filter(Mob.Nav.history(state.nav), keep)) - |> Mob.Nav.map_parked(& &1) + |> Mob.Nav.put_history(Enum.reject(Mob.Nav.history(state.nav), dead?)) + |> Mob.Nav.drop_parked(dead?) %{state | nav: nav} end @@ -604,10 +663,20 @@ defmodule Mob.Screen do # terminate/2 that signal overrides the owner's exit reason, which turns a # clean GenServer.stop(owner, :normal) into an exit with :shutdown. Process.unlink(pid) - GenServer.stop(pid, :shutdown, @stop_timeout_ms) + + try do + GenServer.stop(pid, :shutdown, @stop_timeout_ms) + catch + # A wedged screen ignores the shutdown request. GenServer.stop/3 kills + # only its own proxy on timeout, so without this the screen survives — + # unlinked, untracked, and still dumping to Mob.ScreenState under the + # same key as its replacement. That is the orphan hazard linking exists + # to prevent. + :exit, _reason -> Process.exit(pid, :kill) + end end - catch - :exit, _reason -> :ok + + :ok end # ── Navigation ──────────────────────────────────────────────────────────── @@ -616,13 +685,12 @@ defmodule Mob.Screen do defp apply_nav_action({:push, dest, params}, state, mode) do {new_module, route_params} = resolve_destination(dest) - ref = Mob.Nav.active_ref(state.nav) mount_params = Map.merge(route_params, params) - case start_screen(new_module, mount_params, ref, state) do + case start_screen(new_module, mount_params, state) do {:ok, entry, state} -> nav = Mob.Nav.put_history(state.nav, [state.current | Mob.Nav.history(state.nav)]) - state = %{state | nav: nav, current: entry} + state = make_current(%{state | nav: nav}, entry) do_paint(entry, :push, state, mode) state @@ -638,7 +706,7 @@ defmodule Mob.Screen do # goes with it. The ones still in `rest` stay resident — that is what # makes pop restore prior state without re-mounting. state = stop_screen(state.current, state) - state = %{state | nav: Mob.Nav.put_history(state.nav, rest), current: previous} + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) do_paint(previous, :pop, state, mode) state @@ -655,7 +723,7 @@ defmodule Mob.Screen do ] state = Enum.reduce(discarded, state, &stop_screen/2) - state = %{state | nav: Mob.Nav.put_history(state.nav, []), current: root} + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, root) do_paint(root, :pop, state, mode) state @@ -672,7 +740,7 @@ defmodule Mob.Screen do {:found, previous, rest} -> discarded = [state.current | Enum.take_while(history, &(&1.pid != previous.pid))] state = Enum.reduce(discarded, state, &stop_screen/2) - state = %{state | nav: Mob.Nav.put_history(state.nav, rest), current: previous} + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) do_paint(previous, :pop, state, mode) state @@ -683,14 +751,13 @@ defmodule Mob.Screen do defp apply_nav_action({:reset, dest, params}, state, mode) do {new_module, route_params} = resolve_destination(dest) - ref = Mob.Nav.active_ref(state.nav) mount_params = Map.merge(route_params, params) - case start_screen(new_module, mount_params, ref, state) do + case start_screen(new_module, mount_params, state) do {:ok, entry, state} -> discarded = [state.current | Mob.Nav.history(state.nav)] state = Enum.reduce(discarded, state, &stop_screen/2) - state = %{state | nav: Mob.Nav.put_history(state.nav, []), current: entry} + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, entry) do_paint(entry, :reset, state, mode) state @@ -702,24 +769,16 @@ defmodule Mob.Screen do defp apply_nav_action({:switch_tab, tab}, state, mode) do case Mob.Nav.switch(state.nav, tab, state.current) do {:switched, nav, entry} -> - # The restored screen already carries the ref of the stack it was - # parked under, which is the one becoming active. - Mob.Sender.set_active(Mob.Nav.active_ref(nav)) - state = %{state | nav: nav, current: entry} + state = make_current(%{state | nav: nav}, entry) do_paint(entry, :none, state, mode) state {:mount_root, nav, root_module} -> - # Start first, mutate after. Switching nav and the sender's active ref - # before the mount could fail leaves the sender addressing a stack whose - # screen never started, and every frame the live screen produces is then - # dropped — a silent freeze. - ref = Mob.Nav.active_ref(nav) - - case start_screen(root_module, %{}, ref, state) do + # Start first, mutate after. Switching nav before the mount could fail + # leaves navigation pointing at a stack whose screen never started. + case start_screen(root_module, %{}, state) do {:ok, entry, state} -> - Mob.Sender.set_active(ref) - state = %{state | nav: nav, current: entry} + state = make_current(%{state | nav: nav}, entry) do_paint(entry, :none, state, mode) state diff --git a/lib/mob/screen/server.ex b/lib/mob/screen/server.ex index b872766c..9950f955 100644 --- a/lib/mob/screen/server.ex +++ b/lib/mob/screen/server.ex @@ -41,6 +41,8 @@ defmodule Mob.Screen.Server do use GenServer + require Logger + @state_sync_interval_ms 30_000 @typedoc "Which navigation stack this screen belongs to, for addressing renders." @@ -78,6 +80,16 @@ defmodule Mob.Screen.Server do @spec socket(pid()) :: Mob.Socket.t() def socket(pid), do: GenServer.call(pid, :get_socket) + @doc """ + Render this screen's tree, in this screen's process. + + For inspection only — it does not commit anything. Running `render/1` in the + caller instead would put user code in the owner, where a raise takes down + navigation and every other screen. + """ + @spec tree(pid()) :: map() + def tree(pid), do: GenServer.call(pid, :get_tree) + @doc "Paint this screen, with the given navigation transition." @spec render(pid(), atom()) :: :ok def render(pid, transition \\ :none), do: GenServer.cast(pid, {:render, transition}) @@ -143,6 +155,10 @@ defmodule Mob.Screen.Server do def handle_call(:get_socket, _from, state), do: {:reply, state.socket, state} + def handle_call(:get_tree, _from, state) do + {:reply, state.module.render(state.socket.assigns), state} + end + def handle_call({:render_sync, transition}, _from, state) do {:reply, :ok, %{state | socket: paint(state, transition, :sync)}} end @@ -200,6 +216,20 @@ defmodule Mob.Screen.Server do end end + # Trapping exits means a linked task's crash arrives here as a message + # instead of killing this screen. Passing it to the user's handle_info would + # silently swallow it — the default clause ignores unknown messages — so say + # so, then let the screen see it in case it wants to react. + def handle_info({:EXIT, pid, reason} = message, state) + when reason != :normal and pid != :erlang.map_get(:owner, state) do + Logger.warning( + "[mob] #{inspect(state.module)}: linked process #{inspect(pid)} exited: " <> + "#{inspect(reason)}" + ) + + forward(message, state) + end + def handle_info(message, state), do: forward(message, state) @impl GenServer diff --git a/lib/mob/test.ex b/lib/mob/test.ex index eceac67f..e9eef78f 100644 --- a/lib/mob/test.ex +++ b/lib/mob/test.ex @@ -238,7 +238,8 @@ defmodule Mob.Test do Block until the app has finished processing and the current frame is on screen. - Drains the navigation owner, then the screen process, then waits for + Drains the navigation owner and the screen process (twice, since an event + that navigates hands off to a *different* screen), then waits for `Mob.Sender` to commit. All three are needed: the owner forwards the event, the screen builds the tree, and the sender commits it — so a drained owner mailbox alone does not mean the frame has been rendered. @@ -253,19 +254,28 @@ defmodule Mob.Test do """ @spec settle(node(), timeout()) :: :ok def settle(node, timeout \\ 5000) do - # Three hops, not two. Since MOB-112 the process registered as :mob_screen - # is the navigation *owner*; it forwards events to the screen, which builds - # the tree, which the sender commits. Draining only the owner proves - # nothing about the other two. + # Since MOB-112 the process registered as :mob_screen is the navigation + # *owner*; it forwards events to the screen, which builds the tree, which + # the sender commits. Draining only the owner proves nothing about the rest. + # + # Twice, because an event that navigates moves through owner -> old screen + # -> owner -> NEW screen. Draining once settles the screen that is on its + # way out and returns before the incoming one has rendered, so a + # tap -> settle -> screenshot would read the stale frame. + drain_owner_and_screen(node) + drain_owner_and_screen(node) + + :rpc.call(node, Mob.Sender, :sync, [timeout]) + :ok + end + + defp drain_owner_and_screen(node) do :rpc.call(node, :sys, :get_state, [:mob_screen]) case :rpc.call(node, Mob.Screen, :get_screen_pid, [:mob_screen]) do pid when is_pid(pid) -> :rpc.call(node, :sys, :get_state, [pid]) _ -> :ok end - - :rpc.call(node, Mob.Sender, :sync, [timeout]) - :ok end # ── System gestures ─────────────────────────────────────────────────────────── diff --git a/test/mob/nav_test.exs b/test/mob/nav_test.exs index 00cba06f..7f987e09 100644 --- a/test/mob/nav_test.exs +++ b/test/mob/nav_test.exs @@ -108,6 +108,55 @@ defmodule Mob.NavTest do end end + describe "drop_parked/2" do + setup do + nav = Nav.from_layout(two_tabs(), HomeScreen) + {:mount_root, nav, _} = Nav.switch(nav, :settings, entry(HomeScreen)) + %{nav: nav} + end + + test "removes a matching entry from a parked stack's history", %{nav: nav} do + doomed = entry(ProfileScreen) + nav = %{nav | parked: %{home: %{current: entry(HomeScreen), history: [doomed]}}} + + nav = Nav.drop_parked(nav, &(&1 == doomed)) + + assert nav.parked[:home].history == [] + refute nav.parked[:home].current == doomed + end + + test "promotes the history head when a stack's current is dropped", %{nav: nav} do + doomed = entry(HomeScreen) + survivor = entry(ProfileScreen) + nav = %{nav | parked: %{home: %{current: doomed, history: [survivor]}}} + + nav = Nav.drop_parked(nav, &(&1 == doomed)) + + assert nav.parked[:home].current == survivor + assert nav.parked[:home].history == [] + end + + test "removes the stack entirely when nothing is left" do + # It must not linger with a dead current — switching to it would restore a + # corpse. Gone from parked means the next switch mounts its root fresh. + doomed = entry(HomeScreen) + nav = Nav.from_layout(two_tabs(), HomeScreen) + nav = %{nav | active: :settings, parked: %{home: %{current: doomed, history: []}}} + + nav = Nav.drop_parked(nav, &(&1 == doomed)) + + refute Map.has_key?(nav.parked, :home) + assert {:mount_root, _nav, HomeScreen} = Nav.switch(nav, :home, entry(SettingsScreen)) + end + + test "leaves non-matching stacks untouched", %{nav: nav} do + kept = entry(HomeScreen) + nav = %{nav | parked: %{home: %{current: kept, history: []}}} + + assert Nav.drop_parked(nav, fn _ -> false end).parked[:home].current == kept + end + end + describe "back_target/1" do test "the first declared stack exits" do assert Nav.back_target(Nav.from_layout(two_tabs(), HomeScreen)) == :exit diff --git a/test/mob/screen/restart_test.exs b/test/mob/screen/restart_test.exs index 8d82bf9b..0f25d6b7 100644 --- a/test/mob/screen/restart_test.exs +++ b/test/mob/screen/restart_test.exs @@ -159,17 +159,26 @@ defmodule Mob.Screen.RestartTest do assert Mob.Screen.get_current_module(owner) == SettingsScreen end - test "is restarted with its OWN stack's ref, not the active one", %{owner: owner} do + test "keeps its own render ref across a restart, and it is never the active one", %{ + owner: owner + } do home = Mob.Screen.get_screen_pid(owner) - assert :sys.get_state(home).ref == :home + home_ref = :sys.get_state(home).ref Mob.Screen.dispatch(owner, "to_settings", %{}) + settings_ref = :sys.get_state(owner).current.ref + refute settings_ref == home_ref + kill_and_settle(owner, home) restarted = parked(owner)[:home].current assert restarted.pid != home - assert restarted.ref == :home, "a parked screen tagged :settings would paint over the tab" - assert :sys.get_state(restarted.pid).ref == :home + # The ref survives the restart — it is the same logical screen — and is + # still not the active one, so a repaint from it is dropped rather than + # committed over the foreground tab. + assert restarted.ref == home_ref + assert :sys.get_state(restarted.pid).ref == home_ref + refute restarted.ref == :sys.get_state(owner).current.ref end test "switching back reaches the restarted screen", %{owner: owner} do @@ -185,6 +194,59 @@ defmodule Mob.Screen.RestartTest do end end + describe "restart ceiling" do + test "a screen that keeps crashing is given up on rather than looped", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + + log = + capture_log(fn -> + # One more than the ceiling. Without it this spins at thousands of + # restarts a second, each writing a log line. + for _ <- 1..7 do + pid = Mob.Screen.get_screen_pid(owner) + Process.exit(pid, :kill) + :sys.get_state(owner) + :sys.get_state(owner) + end + end) + + assert log =~ "given up on rather than restarted in a loop" + end + + test "giving up falls back to the screen beneath", %{owner: owner} do + Mob.Screen.dispatch(owner, "push", %{}) + + capture_log(fn -> + for _ <- 1..7 do + pid = Mob.Screen.get_screen_pid(owner) + Process.exit(pid, :kill) + :sys.get_state(owner) + :sys.get_state(owner) + end + end) + + assert Mob.Screen.get_current_module(owner) == HomeScreen + assert Process.alive?(Mob.Screen.get_screen_pid(owner)) + end + end + + describe "inspection is not a way to kill the app" do + defmodule BadRenderScreen do + use Mob.Screen + def mount(_p, _s, socket), do: {:ok, socket} + def render(_assigns), do: raise("render exploded") + end + + test "a screen whose render/1 raises does not take the owner down", %{owner: owner} do + :ok = GenServer.call(owner, {:navigate, {:reset, BadRenderScreen, %{}}}) + + log = capture_log(fn -> assert GenServer.call(owner, :inspect).tree == nil end) + + assert Process.alive?(owner), "render/1 must run in the screen, not the owner" + assert log =~ "render exploded" + end + end + describe "bookkeeping" do test "a restarted screen is monitored exactly once", %{owner: owner} do Mob.Screen.dispatch(owner, "push", %{}) diff --git a/test/mob/screen_sender_wiring_test.exs b/test/mob/screen_sender_wiring_test.exs index 0ae0177e..771e5f69 100644 --- a/test/mob/screen_sender_wiring_test.exs +++ b/test/mob/screen_sender_wiring_test.exs @@ -4,9 +4,13 @@ defmodule Mob.ScreenSenderWiringTest do Screens run `:no_render` here, so no tree is ever committed — but `Mob.Sender.set_active/1` is a cast, so with a real sender running these - assertions pin *who* declares the active screen and *when*. That matters: - announcing it on every render instead would let any screen promote itself, - which is what disarms the drop-inactive mechanism at MOB-112. + assertions pin *who* declares the active screen and *when*. + + Since MOB-112 the sender's key is per **screen**, not per stack. Every screen + is a live process that repaints on any message it receives, including the ones + below the top of the stack; keyed by stack they would all share a ref, and a + timer tick in a screen the user cannot see would commit its tree over the one + they can. Only the current screen's ref is ever active. """ use ExUnit.Case, async: false @@ -29,6 +33,15 @@ defmodule Mob.ScreenSenderWiringTest do def handle_event("to_nowhere", _, socket), do: {:noreply, Mob.Socket.switch_tab(socket, :not_a_stack)} + + def handle_event("push_detail", _, socket), + do: {:noreply, Mob.Socket.push_screen(socket, Mob.ScreenSenderWiringTest.DetailScreen)} + end + + defmodule DetailScreen do + use Mob.Screen + def mount(_params, _session, socket), do: {:ok, socket} + def render(_assigns), do: %{type: :text, props: %{text: "detail"}, children: []} end defmodule SettingsScreen do @@ -59,6 +72,10 @@ defmodule Mob.ScreenSenderWiringTest do end defp active, do: :sys.get_state(Process.whereis(Sender)).active + defp current_ref(owner), do: :sys.get_state(owner).current.ref + + defp history_refs(owner), + do: owner |> :sys.get_state() |> Map.fetch!(:nav) |> Mob.Nav.history() |> Enum.map(& &1.ref) setup do for name <- [Sender, Mob.Nav.Registry] do @@ -90,36 +107,51 @@ defmodule Mob.ScreenSenderWiringTest do :exit, _ -> :ok end - test "mounting a screen declares its stack active", %{screen: _} do + test "mounting a screen makes that screen active", %{screen: screen} do Sender.sync() - assert active() == :home + assert active() == current_ref(screen) end test "switching stacks moves the active screen", %{screen: screen} do + home_ref = current_ref(screen) + Mob.Screen.dispatch(screen, "to_settings", %{}) Sender.sync() - assert active() == :settings + settings_ref = current_ref(screen) + assert active() == settings_ref + refute settings_ref == home_ref Mob.Screen.dispatch(screen, "to_home", %{}) Sender.sync() - assert active() == :home + assert active() == home_ref, "switching back restores the same screen, so the same ref" end test "an ordinary re-render does not change the active screen", %{screen: screen} do Mob.Screen.dispatch(screen, "to_settings", %{}) Sender.sync() - assert active() == :settings + settings_ref = current_ref(screen) - # A screen re-rendering must not promote itself — this is the assertion that - # fails if set_active/1 moves back into do_render/4. + # A screen re-rendering must not promote itself — this fails if set_active/1 + # moves back into the render path. Mob.Screen.dispatch(screen, "bump", %{}) Sender.sync() - assert active() == :settings + assert active() == settings_ref end test "a switch to an undeclared stack leaves the active screen alone", %{screen: screen} do + home_ref = current_ref(screen) Mob.Screen.dispatch(screen, "to_nowhere", %{}) Sender.sync() - assert active() == :home + assert active() == home_ref + end + + test "a screen below the top of the stack has a different ref", %{screen: screen} do + # The regression this guards: keyed by stack, a background screen's timer + # tick would commit its tree over the foreground screen, tap table included. + Mob.Screen.dispatch(screen, "push_detail", %{}) + Sender.sync() + [below] = history_refs(screen) + assert active() == current_ref(screen) + refute below == current_ref(screen) end end From 87e800437f0bd46046c382db5ff99bbca2c02a23 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 29 Aug 2026 00:01:27 -0600 Subject: [PATCH 4/5] MOB-112: fix the two blockers from review #3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review #3 cleared the model review #2's fixes introduced — I could not make Mob.Sender's active ref disagree with state.current.ref down any path or race, including a screen that navigates then crashes, and a push processed before a crashed screen's EXIT. Two blockers remained, both small, both in code that predates the ref work. 1. A bad navigation destination raised inside the OWNER. push_screen/2 takes any atom and resolve_destination/1 raises on an unregistered one, so a typo killed navigation and every screen — falsifying the isolation this module's moduledoc and the ADR both assert, in the same commit that asserts it. It is caught now: logged, navigation untouched, current screen repaints. Same guard on decode_notification_json/1, which runs in the owner because a launch notification comes from native rather than a screen, and :json.decode/1 raises on malformed input. The existing test asserted the crash. It now asserts the app survives. 2. Giving up on a screen bricked a tab-bar app. recover_from_failed_restart/2's last branch fired when the current screen could not come back and its own stack had nothing beneath it — which in a tab-bar app is the ORDINARY shape, since every tab root has an empty history. The owner kept a dead pid as current, every later event was safe_call'd into a corpse and replied :ok, and the sender still pointed at the dead ref, all while a live screen sat parked under another tab. It now switches to a parked stack and drops the dead entry so nothing can restore a corpse later. Also from review #3: state.restarts grew for the owner's lifetime (nothing removed a key), pruned in stop_screen/2; and the types and prose describing the model this change replaced — Mob.Nav's `entry :: {module, socket}`, Mob.Screen.Server's `render_ref :: atom()` still calling itself a stack, and Mob.Sender's "MOB-112 replaces it with a per-screen reference" in the commit that is MOB-112. Mob.Nav.active_ref/1 lost its last caller when the render ref became per-screen and is removed rather than left as dead public API. Two findings are deferred to MOB-121 with the reasoning written down: the owner is now a shared serialisation point (:infinity calls mean one wedged screen blocks every other), and stopping a wedged screen costs 5s that pop_to_root and reset pay serially. Neither is a regression — before MOB-112 there was one process to block — but both change shape with N screens. Tests: 2 more, both verified as negative controls. Suite 1233 passed, format and credo --strict clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...-08-28-screen-processes-and-supervision.md | 40 ++++- lib/mob/nav.ex | 25 ++- lib/mob/screen.ex | 160 +++++++++++++----- lib/mob/screen/server.ex | 15 +- lib/mob/sender.ex | 9 +- test/mob/nav/screen_nav_test.exs | 28 +-- test/mob/screen/restart_test.exs | 48 ++++++ 7 files changed, 241 insertions(+), 84 deletions(-) diff --git a/decisions/2026-08-28-screen-processes-and-supervision.md b/decisions/2026-08-28-screen-processes-and-supervision.md index 3ff6e5d7..759bd491 100644 --- a/decisions/2026-08-28-screen-processes-and-supervision.md +++ b/decisions/2026-08-28-screen-processes-and-supervision.md @@ -1,4 +1,4 @@ -# Screen processes: one per screen, owned and monitored rather than supervised +# Screen processes: one per screen, owned and linked rather than supervised - Date: 2026-08-28 - Status: accepted @@ -95,7 +95,7 @@ replacement. in `handle_event` exits that inner call, which would have killed the owner — defeating the isolation on the one path MOB-112's acceptance names explicitly. Every owner-to-screen call goes through `safe_call/1`, which catches the exit -and lets the monitor repair the screen. +and lets the exit signal repair the screen. ### A navigation entry carries what a restart needs @@ -126,6 +126,28 @@ Background screens are re-mounted eagerly rather than lazily. A crash is rare, and keeping every nav entry a live pid means popping or switching back never has to handle a corpse. +### A bad navigation destination is ignored, not fatal + +`push_screen/2` takes any atom and `resolve_destination/1` raises on one that +was never registered — a typo reaches it. That raise runs in the *owner*, so it +would kill navigation and every screen, falsifying the isolation this module's +moduledoc promises. It is caught: the destination is logged, navigation is left +untouched, and the current screen repaints. + +The same reasoning covers `decode_notification_json/1`, which runs in the owner +because a launch notification comes from native rather than from a screen. +`:json.decode/1` raises on malformed input. + +### Giving up on a screen falls back to a live tab + +When a screen cannot be re-mounted and its own stack has nothing beneath it, +the owner switches to a parked stack rather than keeping a dead pid as +`current`. In a tab-bar app "nothing beneath it" is the *ordinary* shape — +every tab root has an empty history — so without this a repeat crash on one tab +bricks the whole app while a perfectly good screen sits parked under another. +The dead entry is dropped from `parked` immediately after the switch, so +nothing can restore a corpse later. + ### A no-op navigation still paints The screen deliberately does not paint when it produced a nav action, so every @@ -157,7 +179,7 @@ history stay alive, which is what makes pop restore prior state without re-mounting — and what the epic's ADR called out as the memory cost of matching how iOS and Android actually behave. -Demonitoring before stopping matters: without it, the shutdown the owner asked +Unlinking before stopping matters: without it, the shutdown the owner asked for returns as a `:DOWN` and the screen is "restarted" immediately after being deliberately discarded. @@ -193,6 +215,18 @@ not yank the stack out from under what the user is looking at. plugin messages — is forwarded by the owner to the active screen. - An owner-to-screen call returns `nil` rather than raising when the screen is mid-crash. `get_socket/1` and `Mob.Test.assigns/1` document and handle that. +- **Known, deferred, and worth an issue of their own.** The owner is now a + shared serialisation point: `dispatch/3` passes `:infinity`, so one screen + wedged in a callback blocks the owner — and therefore the back gesture, + notifications, and message routing for *every other* screen — for as long as + it takes. Stopping a wedged screen costs the owner a 5s timeout before it + resorts to a kill, and `pop_to_root`/`reset` pay that serially per discarded + screen. Both are defensible as written for one screen and questionable for N; + neither is a regression from master, where there was only one process to + block. +- **A tripped restart ceiling is permanent and silent to the user.** A screen + crashing repeatedly for a transient reason is dropped for the process + lifetime, with only a log line. No cooldown, no retry. - **`Mob.Test.settle/2` now drains three processes, not two.** `:mob_screen` is the navigation owner; it forwards to the screen, which builds the tree, which the sender commits. Draining only the owner proved nothing — every diff --git a/lib/mob/nav.ex b/lib/mob/nav.ex index 2aec87c3..87d7a55d 100644 --- a/lib/mob/nav.ex +++ b/lib/mob/nav.ex @@ -38,9 +38,13 @@ defmodule Mob.Nav do first visit its state is retained for the lifetime of the app. """ - alias Mob.Socket + @typedoc """ + Whatever the caller uses to identify a screen. Opaque here. - @type entry :: {module(), Socket.t()} + `Mob.Screen` puts `%{module:, pid:, params:, ref:}` in these slots since + MOB-112 — this module never looks inside one. + """ + @type entry :: term() @type stack_name :: atom() @type parked_stack :: %{current: entry(), history: [entry()]} @@ -130,17 +134,6 @@ defmodule Mob.Nav do @spec active(t()) :: stack_name() | nil def active(%__MODULE__{active: active}), do: active - @doc """ - A stable identifier for the active stack, for addressing renders. - - Falls back to `:__mob_single__` when the app declared no layout, so the sender - always has a concrete screen to compare against rather than a `nil` that would - match nothing. - """ - @spec active_ref(t()) :: stack_name() - def active_ref(%__MODULE__{active: nil}), do: :__mob_single__ - def active_ref(%__MODULE__{active: active}), do: active - @doc "Declared stack names, in declaration order." @spec stacks(t()) :: [stack_name()] def stacks(%__MODULE__{order: order}), do: order @@ -166,6 +159,12 @@ defmodule Mob.Nav do %{nav | parked: parked} end + @doc "Names of the stacks currently parked, in declaration order." + @spec parked_stacks(t()) :: [stack_name()] + def parked_stacks(%__MODULE__{parked: parked, order: order}) do + Enum.filter(order, &Map.has_key?(parked, &1)) + end + @doc """ Remove every parked entry for which `fun` returns true. diff --git a/lib/mob/screen.ex b/lib/mob/screen.ex index e591c269..a40e767f 100644 --- a/lib/mob/screen.ex +++ b/lib/mob/screen.ex @@ -408,7 +408,22 @@ defmodule Mob.Screen do # A notification that launched the app from a killed state. def handle_info({:mob_launch_notification, json}, state) do - handle_info({:notification, decode_notification_json(json)}, state) + # Decoded here rather than in the screen because the payload comes from + # native, not from a screen. :json.decode/1 raises on malformed input, and + # this runs in the owner — so a bad payload would take down every screen. + notification = + try do + decode_notification_json(json) + rescue + error -> + Logger.error( + "[mob] launch notification could not be decoded: " <> Exception.message(error) + ) + + %{source: :local, data: %{}} + end + + handle_info({:notification, notification}, state) end # System back gesture (Android hardware/swipe, iOS edge-pan). Handled here so @@ -598,9 +613,29 @@ defmodule Mob.Screen do state true -> + fall_back_to_parked_stack(entry, state) + end + end + + # "Nothing beneath it" is the *common* shape in a tab-bar app — every tab root + # has an empty history — and there is usually a live screen parked under + # another tab. Leaving a dead pid as `current` bricks the app: every later + # event is safe_call'd into a corpse and replies :ok, while a perfectly good + # screen sits parked one call away. + defp fall_back_to_parked_stack(entry, state) do + with [name | _] <- Mob.Nav.parked_stacks(state.nav), + {:switched, nav, live} <- Mob.Nav.switch(state.nav, name, entry) do + # switch/3 parks the dead entry under the outgoing stack; drop it straight + # after, so nothing can restore a corpse by switching back. + nav = Mob.Nav.drop_parked(nav, &(&1.pid == entry.pid)) + state = make_current(%{state | nav: nav}, live) + paint(live, :pop, state) + state + else + _ -> Logger.error( - "[mob] #{inspect(entry.module)} could not be restarted and there is no screen " <> - "beneath it. The app has no live screen." + "[mob] #{inspect(entry.module)} could not be restarted and there is no other live " <> + "screen to fall back to. The app has no live screen." ) state @@ -650,8 +685,15 @@ defmodule Mob.Screen do # Drop the entry from tracking BEFORE stopping, so the exit we asked for is # recognised as deliberate rather than restarted as a crash. - defp stop_screen(%{pid: pid}, state) do - state = %{state | screens: Map.delete(state.screens, pid)} + defp stop_screen(%{pid: pid} = entry, state) do + # Forget the restart history too — the map is keyed by screen ref and + # nothing else ever removes a key, so it grows for the owner's lifetime. + state = %{ + state + | screens: Map.delete(state.screens, pid), + restarts: Map.delete(state.restarts, entry.ref) + } + stop_process(pid) state end @@ -684,18 +726,8 @@ defmodule Mob.Screen do defp apply_nav_action(nil, state, _mode), do: state defp apply_nav_action({:push, dest, params}, state, mode) do - {new_module, route_params} = resolve_destination(dest) - mount_params = Map.merge(route_params, params) - - case start_screen(new_module, mount_params, state) do - {:ok, entry, state} -> - nav = Mob.Nav.put_history(state.nav, [state.current | Mob.Nav.history(state.nav)]) - state = make_current(%{state | nav: nav}, entry) - do_paint(entry, :push, state, mode) - state - - {:error, _reason} -> - repaint_current(state, mode) + with {:ok, new_module, route_params} <- safe_resolve(dest, state) do + push_resolved(new_module, Map.merge(route_params, params), state, mode) end end @@ -733,36 +765,14 @@ defmodule Mob.Screen do end defp apply_nav_action({:pop_to, dest}, state, mode) do - target = resolve_module(dest) - history = Mob.Nav.history(state.nav) - - case pop_to_module(history, target) do - {:found, previous, rest} -> - discarded = [state.current | Enum.take_while(history, &(&1.pid != previous.pid))] - state = Enum.reduce(discarded, state, &stop_screen/2) - state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) - do_paint(previous, :pop, state, mode) - state - - :not_found -> - repaint_current(state, mode) + with {:ok, target, _params} <- safe_resolve(dest, state) do + pop_to_resolved(target, state, mode) end end defp apply_nav_action({:reset, dest, params}, state, mode) do - {new_module, route_params} = resolve_destination(dest) - mount_params = Map.merge(route_params, params) - - case start_screen(new_module, mount_params, state) do - {:ok, entry, state} -> - discarded = [state.current | Mob.Nav.history(state.nav)] - state = Enum.reduce(discarded, state, &stop_screen/2) - state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, entry) - do_paint(entry, :reset, state, mode) - state - - {:error, _reason} -> - repaint_current(state, mode) + with {:ok, new_module, route_params} <- safe_resolve(dest, state) do + reset_resolved(new_module, Map.merge(route_params, params), state, mode) end end @@ -791,6 +801,49 @@ defmodule Mob.Screen do end end + defp push_resolved(new_module, mount_params, state, mode) do + case start_screen(new_module, mount_params, state) do + {:ok, entry, state} -> + nav = Mob.Nav.put_history(state.nav, [state.current | Mob.Nav.history(state.nav)]) + state = make_current(%{state | nav: nav}, entry) + do_paint(entry, :push, state, mode) + state + + {:error, _reason} -> + repaint_current(state, mode) + end + end + + defp reset_resolved(new_module, mount_params, state, mode) do + case start_screen(new_module, mount_params, state) do + {:ok, entry, state} -> + discarded = [state.current | Mob.Nav.history(state.nav)] + state = Enum.reduce(discarded, state, &stop_screen/2) + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, entry) + do_paint(entry, :reset, state, mode) + state + + {:error, _reason} -> + repaint_current(state, mode) + end + end + + defp pop_to_resolved(target, state, mode) do + history = Mob.Nav.history(state.nav) + + case pop_to_module(history, target) do + {:found, previous, rest} -> + discarded = [state.current | Enum.take_while(history, &(&1.pid != previous.pid))] + state = Enum.reduce(discarded, state, &stop_screen/2) + state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous) + do_paint(previous, :pop, state, mode) + state + + :not_found -> + repaint_current(state, mode) + end + end + # A nav action that changed nothing still has to paint. The screen deliberately # does not paint when it produced an action, so without this the assigns it set # in the same callback would never reach the screen — re-tapping the active tab @@ -800,9 +853,24 @@ defmodule Mob.Screen do state end - defp resolve_module(dest) when is_atom(dest) do - {module, _route_params} = resolve_destination(dest) - module + # `push_screen/2` and friends take any atom, and an unregistered one raises. + # That raise runs in the OWNER, where it would kill navigation and every + # screen — falsifying the isolation this module's moduledoc promises, over a + # typo. A bad destination leaves navigation untouched and repaints instead. + # + # Returns the state unchanged (via repaint_current/2) rather than an error + # tuple, so the `with` in each caller falls straight through. + defp safe_resolve(dest, state) do + {module, route_params} = resolve_destination(dest) + {:ok, module, route_params} + rescue + error -> + Logger.error( + "[mob] navigation to #{inspect(dest)} failed and was ignored: " <> + Exception.message(error) + ) + + repaint_current(state, :async) end # Resolves a navigation destination to {module, route_params}. A loaded diff --git a/lib/mob/screen/server.ex b/lib/mob/screen/server.ex index 9950f955..6f378425 100644 --- a/lib/mob/screen/server.ex +++ b/lib/mob/screen/server.ex @@ -45,16 +45,23 @@ defmodule Mob.Screen.Server do @state_sync_interval_ms 30_000 - @typedoc "Which navigation stack this screen belongs to, for addressing renders." - @type render_ref :: atom() + @typedoc """ + Identifies this screen to `Mob.Sender`. Unique per **screen**, not per stack. + + Every screen is a live process that repaints on any message it receives, + including the ones below the top of a stack. Keyed by stack they would share + a ref, and a timer tick in a screen the user cannot see would commit its tree + over the one they can. The sender only commits the active ref. + """ + @type render_ref :: reference() defstruct [:module, :socket, :render_mode, :ref, :owner] @doc """ Start a screen linked to the calling process. - `:owner` receives nav actions and the exit signal. `:ref` is the navigation - stack this screen belongs to, used to address its renders at `Mob.Sender`. + `:owner` receives nav actions and the exit signal. `:ref` identifies this + screen to `Mob.Sender` and is unique per screen — see `t:render_ref/0`. `Mob.Screen` links *and* traps exits. Linking alone would make the owner die with any screen it stopped or that crashed; trapping alone would leave every diff --git a/lib/mob/sender.ex b/lib/mob/sender.ex index d02cd634..35b7c74e 100644 --- a/lib/mob/sender.ex +++ b/lib/mob/sender.ex @@ -61,11 +61,12 @@ defmodule Mob.Sender do require Logger @typedoc """ - Identifies which screen a tree belongs to. Today this is the active - navigation stack's name (see `Mob.Nav.active_ref/1`); MOB-112 replaces it with - a per-screen reference. + Identifies which screen a tree belongs to — one per live screen since + MOB-112, not one per navigation stack. Screens below the top of a stack are + live processes that repaint, so a stack-wide key would let a background + screen's tree commit over the foreground one. """ - @type screen_ref :: atom() | reference() + @type screen_ref :: reference() | atom() defstruct active: nil, pending: %{} diff --git a/test/mob/nav/screen_nav_test.exs b/test/mob/nav/screen_nav_test.exs index 5ea302f8..4ce30577 100644 --- a/test/mob/nav/screen_nav_test.exs +++ b/test/mob/nav/screen_nav_test.exs @@ -1,6 +1,8 @@ defmodule Mob.Nav.ScreenNavTest do use ExUnit.Case, async: false + import ExUnit.CaptureLog + # ── Screen fixtures ──────────────────────────────────────────────────────── # Bare module names inside nested defmodule blocks don't auto-alias to siblings. # Use module attributes with fully qualified names for cross-screen references. @@ -283,21 +285,19 @@ defmodule Mob.Nav.ScreenNavTest do end end - test "raises ArgumentError for unregistered atom" do + test "an unregistered atom is logged and ignored, not fatal" do + # Before MOB-112 this raised in the one screen process, which was also the + # whole app. The raise now happens in the navigation *owner*, where it + # would take down every screen — over a typo in push_screen/2. It is + # caught: navigation is left untouched and the app carries on. {:ok, pid} = Mob.Screen.start_link(UnknownNavScreen, %{}) - # Unlink so the server crash doesn't kill the test process — we only want - # to observe the exit that GenServer.call propagates through the call path. - Process.unlink(pid) - - exit_reason = - try do - Mob.Screen.dispatch(pid, "bad_nav", %{}) - nil - catch - :exit, reason -> reason - end - - assert inspect(exit_reason) =~ "no_such_screen" + on_exit(fn -> if Process.alive?(pid), do: GenServer.stop(pid) end) + + log = capture_log(fn -> assert :ok = Mob.Screen.dispatch(pid, "bad_nav", %{}) end) + + assert log =~ "no_such_screen" + assert Process.alive?(pid) + assert Mob.Screen.get_current_module(pid) == UnknownNavScreen end end end diff --git a/test/mob/screen/restart_test.exs b/test/mob/screen/restart_test.exs index 0f25d6b7..8f81ad41 100644 --- a/test/mob/screen/restart_test.exs +++ b/test/mob/screen/restart_test.exs @@ -230,6 +230,54 @@ defmodule Mob.Screen.RestartTest do end end + describe "giving up on a tab root" do + test "falls back to a live parked tab instead of bricking the app", %{owner: owner} do + # Every tab root has an empty history, so "nothing beneath it" is the + # ordinary shape here, not an exotic one. Leaving the dead screen as + # current would strand the app with a perfectly good :home tab parked. + home = Mob.Screen.get_screen_pid(owner) + Mob.Screen.dispatch(owner, "to_settings", %{}) + assert Mob.Screen.get_current_module(owner) == SettingsScreen + assert Mob.Nav.history(:sys.get_state(owner).nav) == [] + + log = + capture_log(fn -> + # One more than @max_restarts trips the ceiling. Going further would + # start killing the screen we just fell back to. + for _ <- 1..6 do + Process.exit(Mob.Screen.get_screen_pid(owner), :kill) + :sys.get_state(owner) + :sys.get_state(owner) + end + end) + + assert log =~ "given up on" + assert Mob.Screen.get_current_module(owner) == HomeScreen + assert Mob.Screen.get_screen_pid(owner) == home + assert Process.alive?(home) + assert Mob.Screen.get_socket(owner).assigns.where == :home + end + + test "the dead screen is not left parked for a later switch to restore", %{owner: owner} do + Mob.Screen.dispatch(owner, "to_settings", %{}) + dead = Mob.Screen.get_screen_pid(owner) + + capture_log(fn -> + for _ <- 1..6 do + Process.exit(Mob.Screen.get_screen_pid(owner), :kill) + :sys.get_state(owner) + :sys.get_state(owner) + end + end) + + # Switching back must mount a fresh root, never restore the corpse. + Mob.Screen.dispatch(owner, "to_settings", %{}) + assert Mob.Screen.get_current_module(owner) == SettingsScreen + assert Mob.Screen.get_screen_pid(owner) != dead + assert Process.alive?(Mob.Screen.get_screen_pid(owner)) + end + end + describe "inspection is not a way to kill the app" do defmodule BadRenderScreen do use Mob.Screen From ac64109e5e6d27143de541dea93ea1356b61e4af Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 29 Aug 2026 00:02:59 -0600 Subject: [PATCH 5/5] MOB-112: make the crash-test helpers deterministic My own test was flaky, and review #3 predicted exactly this: kill_and_settle/2 used :sys.get_state/1 as a barrier, which is not ordered against Process.exit(pid, :kill). A later kill could land on an already-dead pid and be a no-op, so the loop performed fewer restarts than intended and the ceiling sometimes did not trip. Caught it on run 4 of 8. Each kill now waits for the :DOWN before draining the owner, so every iteration lands on a live process. 12/12 clean runs of the restart file, 10/10 clean full-suite runs. Co-Authored-By: Claude Opus 5 (1M context) --- test/mob/screen/restart_test.exs | 45 ++++++++++++-------------------- 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/test/mob/screen/restart_test.exs b/test/mob/screen/restart_test.exs index 8f81ad41..0293c0f8 100644 --- a/test/mob/screen/restart_test.exs +++ b/test/mob/screen/restart_test.exs @@ -73,13 +73,20 @@ defmodule Mob.Screen.RestartTest do defp history(owner), do: owner |> owner_state() |> Map.fetch!(:nav) |> Mob.Nav.history() defp parked(owner), do: owner |> owner_state() |> Map.fetch!(:nav) |> Map.fetch!(:parked) + # :sys.get_state/1 alone is not a barrier here — it is not ordered against the + # kill, so a later kill can land on an already-dead pid and be a no-op. + # Waiting for the :DOWN makes each kill land on a live process. defp kill_and_settle(owner, pid) do - capture_log(fn -> - Process.exit(pid, :kill) - # Let the owner process the EXIT and finish the restart. - :sys.get_state(owner) - :sys.get_state(owner) - end) + capture_log(fn -> kill_and_wait(owner, pid) end) + end + + defp kill_and_wait(owner, pid) do + ref = Process.monitor(pid) + Process.exit(pid, :kill) + assert_receive {:DOWN, ^ref, :process, ^pid, _} + # Then let the owner see the EXIT and finish the restart. + :sys.get_state(owner) + :sys.get_state(owner) end setup do @@ -202,12 +209,7 @@ defmodule Mob.Screen.RestartTest do capture_log(fn -> # One more than the ceiling. Without it this spins at thousands of # restarts a second, each writing a log line. - for _ <- 1..7 do - pid = Mob.Screen.get_screen_pid(owner) - Process.exit(pid, :kill) - :sys.get_state(owner) - :sys.get_state(owner) - end + for _ <- 1..6, do: kill_and_wait(owner, Mob.Screen.get_screen_pid(owner)) end) assert log =~ "given up on rather than restarted in a loop" @@ -217,12 +219,7 @@ defmodule Mob.Screen.RestartTest do Mob.Screen.dispatch(owner, "push", %{}) capture_log(fn -> - for _ <- 1..7 do - pid = Mob.Screen.get_screen_pid(owner) - Process.exit(pid, :kill) - :sys.get_state(owner) - :sys.get_state(owner) - end + for _ <- 1..6, do: kill_and_wait(owner, Mob.Screen.get_screen_pid(owner)) end) assert Mob.Screen.get_current_module(owner) == HomeScreen @@ -244,11 +241,7 @@ defmodule Mob.Screen.RestartTest do capture_log(fn -> # One more than @max_restarts trips the ceiling. Going further would # start killing the screen we just fell back to. - for _ <- 1..6 do - Process.exit(Mob.Screen.get_screen_pid(owner), :kill) - :sys.get_state(owner) - :sys.get_state(owner) - end + for _ <- 1..6, do: kill_and_wait(owner, Mob.Screen.get_screen_pid(owner)) end) assert log =~ "given up on" @@ -263,11 +256,7 @@ defmodule Mob.Screen.RestartTest do dead = Mob.Screen.get_screen_pid(owner) capture_log(fn -> - for _ <- 1..6 do - Process.exit(Mob.Screen.get_screen_pid(owner), :kill) - :sys.get_state(owner) - :sys.get_state(owner) - end + for _ <- 1..6, do: kill_and_wait(owner, Mob.Screen.get_screen_pid(owner)) end) # Switching back must mount a fresh root, never restore the corpse.