fix(lua): budget the eval heap against the callback, not the state it was handed by Taure · Pull Request #538 · widgrensit/asobi

Taure · 2026-08-21T08:22:08Z

Closes #536.

The reporter's zones stop ticking for seconds at a time, hostiles stop moving
and joins fail, with one player in one zone. They traced it to the per-eval
heap cap killing zone_tick on repeat, and they were right about the cap. The
measurements say the cap is the second problem.

The cap bounded the state, not the callback

bounded_eval/2 spawned its worker with an absolute max_heap_size, and the
spawn itself copies the persistent Luerl state into that worker. So the cap
bounded the size of the state - which the code's own comment says is where
large state is supposed to live - rather than what the callback allocated. A
handler that allocated nothing was killed once the state behind it was large
enough, and killed again on every tick until the collector next ran.

The worker now measures its own heap after the copy and caps at
2 x Base + Budget. Two things measurement forced that reasoning alone got
wrong:

The state has to be counted twice. Collecting an all-live heap copies it, so
Base + Budget kills a large state on its own first GC - the same bug with
different arithmetic. Verified: every state >= 13 MB died.
{fullsweep_after, 0} is what holds that to one copy instead of an
unbounded number of generational ones. It is also faster, not slower:
132 ms -> 64 ms per callback at a 13 MB state, 218 ms -> 98 ms at 39 MB. The
worker's heap is one big live term plus fresh garbage; generational
collection has nothing to be clever about.

The copy is the first problem

A callback that does nothing at all, timed 20x:

state	`call/4` (worker + copy)	`call/3` (inline)	`luerl:gc/1`
0.4 MB	1.81 ms	0.003 ms	0.1 ms
6 MB	41.19 ms	0.005 ms	1.6 ms
24 MB	101.63 ms	0.006 ms	5.7 ms
62 MB	418.19 ms	0.004 ms	14.3 ms

Roughly 7 ms per MB. At the 400-690 MB the reporter measured that is three to
five seconds per callback before the script runs a line, which is the
"world is empty" symptom on its own, independent of any kill.

Two consequences here, and a third proposed separately:

The collector's budget was the wrong yardstick. A flat 5 ms ceiling
decided whether a collection was worth it, but what a collection competes
with is that copy, not a fixed number. A 15 ms collection that shrinks a
60 MB state saves ~380 ms on every callback until the next one. The flat
number backed the interval off hardest on exactly the states that could least
afford it, which is the sawtooth in the issue. The budget scales with the
state now, capped at a quarter of the abandon ceiling so the back-off stays
reachable.
Collection runs before the callback, so the copy is of the collected
state. It also breaks a wedge the collector could not: a callback that failed
on heap or timeout never reached a collection placed after it, so the next
tick failed identically, forever.
Removing the copy entirely is ADR 0015 on spike/lua-vm-process -
proposed, not in this PR. The spike measures it flat at 0.1-1 ms against
75 MB, but it costs the guard, so it wants its own decision.

Making it visible

[asobi, lua, state] reports words/bytes per bridge, about once a second,
with kind and the world/match id and coords. asobi also logs lua_state_large
once per excursion past state_warn_words. Before this the only way to see any
of it was to walk the term by hand in a remote shell - which is what the issue
had to do. ADR 0005 gains the event, additive only.

state_warn_words and state_sample_interval_ms are configuration rather than
literals: what counts as a state too large to copy depends on the tick budget,
which the loader cannot see. 100 MB is catastrophic at 80 Hz and merely
expensive at 2 Hz.

A security fix that came out of the review

collect/2 anchored game_state in _G with the metamethod-honouring setter,
and unanchor clears the key each time - so a script's _G __newindex fired
on every collection, on the bridge gen_server, outside bounded_eval. No
wall-clock budget, no reduction budget, no heap cap. setmetatable(_G, ...) is
explicitly permitted by the trust model.

The dangerous form is not the obvious one. A __newindex that never returns
wedges the zone permanently (measured: 4.7 billion reductions, never returned).
But the likely one is an ordinary strict-globals metatable that raises:
set_table_keys/3 catches it, the anchor silently fails, every collection is
skipped - and because a skipped collection costs no time, the adaptive interval
reads it as cheap and drives itself to its minimum. Measured over 400 ticks:
8417 live tables against 11 for the same script without the metatable, with
enabled => true throughout and nothing in the logs. That is #536 re-opened
invisibly by one ordinary line of Lua.

The anchor is asobi's bookkeeping, not the script's data, so it is written raw
now. Pre-existing from #426, but it lives in the code this PR rewrites and this
PR removed the cap that bounded its fallout. luerl_heap is internal to luerl,
hence the patch-range pin on the dep - asobi already depends on luerl_lib the
same way.

What this deliberately does not add

An absolute ceiling on eval memory. It is worth being precise about why, because
it looks like a regression and is not: the old cap never bounded the
persistent state. It killed the eval worker; the state lived in the parent
gen_server and survived untouched. A ceiling therefore converts "node runs out
of memory" into "zone stops ticking forever and the node still holds the
state" - which is #536 by construction, at a different number. The reporter
reached 4.5 GB in one zone with the old cap in place.

The right backstop is a ceiling on the state that stops the zone, so the
supervisor rebuilds a clean VM from asobi_zone_snapshotter's last snapshot.
That is a separate change and needs its own decision, not least because matches
have no equivalent restore path.

Upstream

Two Luerl defects found on the way, both filed with measurements and tests:
rvirding/luerl#226 (the collector's seen set is an ordset against a map-backed
store, so mark and sweep are quadratic - 6x to 289x) and rvirding/luerl#227
(put_private/3 values are not GC roots, a use-after-free). Neither is a
dependency of this PR. If #226 lands, the lua_gc_abandoned path here becomes
near-dead: the live set that trips it today collects in 31 ms patched.

Verification

eunit 2220/0. 22 new tests, including one per bridge-identity call site (the
world one fails without its fix) and both _G-metatable cases (hand-mutated
against the pre-fix collector to confirm they discriminate rather than merely
pass).
CT: asobi_lua, zone_snapshotter, zone_spawner, spatial, match,
lua_storage - 52/52 against Docker Postgres.
dialyzer, xref, rebar3 fmt --check clean. elp lint clean on everything
touched. eqwalizer unchanged from main's baseline (loader 5, world 1, match 0,
telemetry 1). rebar3 ex_doc no new warnings.

rebar3 mutate --diff origin/main, scoped to the four changed modules.
Whole-tree is 11206 mutants and ~40 minutes, which is why CI keeps it off,
but the diff form is cheap and it earned its place twice: it showed that the
bridge-identity and telemetry tests were invisible to per-module tooling
(the plugin pairs a module with <module>_tests, and they lived in their
own module), and it caught a tautology - a rate-limit test that passed
because the measurement had been consumed rather than because the interval
suppressed anything.

module	before	after
`asobi_telemetry`	0.0% (0/6)	100.0% (6/6)
`asobi_lua_world`	44.4% (4/9)	77.8% (7/9)
`asobi_lua_match`	50.0% (2/4)	75.0% (3/4)
`asobi_lua_loader`	50.0% (97/194)	51.5% (100/194)
overall	40.4% (86/213)	54.5% (116/213)

The loader is the weak one and stays that way on purpose: nearly all its
surviving mutants are in the two config accessors and their default
constants. A test pinning 1000 to 1000 is a test of the literal.

The anchor change was additionally mutation-tested by hand - rebuilt with the
pre-fix collect/2, at which point both _G-metatable tests fail.

Reviewed by the architecture guardian, the BEAM security reviewer and the
Erlang code reviewer; all findings applied. Each of the three found a real
defect the previous one had not.

… was handed #536. `bounded_eval` spawned its worker with an absolute `max_heap_size`, but the spawn itself copies the persistent Luerl state into that worker - so the cap bounded the state rather than the callback's own allocation. A handler that allocated nothing was killed once the state behind it was large enough, and killed again on every tick until the collector next ran. That is the opposite of what the code says it does, and it made the budget unusable as the safety valve it is meant to be: it could not tell a runaway script apart from an ordinary one attached to a large state. The cap is now set from inside the worker, measured from its heap after the copy. The state is counted twice on purpose - collecting an all-live heap copies it, so `Base + Budget` would kill a large state on its own first GC - and `fullsweep_after = 0` keeps that at one copy rather than an unbounded number of generational ones. That is also faster: 132ms -> 64ms per callback at a 13MB state, 218ms -> 98ms at 39MB. Two things the fix on its own would leave in place: - The copy is the dominant cost of a Lua tick and nothing about it was visible. Measured, for a callback that does nothing at all: 1.8ms against a 0.4MB state, 41ms against 6MB, 418ms against 62MB - roughly 7ms per MB. So the collector's flat 5ms budget was the wrong yardstick; what a collection competes with is that copy, not a fixed ceiling. The budget now scales with the state, which stops the interval backing off hardest on exactly the states that can least afford it. - Collection happens before the callback rather than after it, so the copy is of the collected state. It also breaks the wedge the collector could not: a callback that failed on heap or timeout never reached a collection placed after it, so the next tick failed the same way, forever. `[asobi, lua, state]` reports the measured size, and asobi logs `lua_state_large` once per state past ~100MB. Neither existed before; the only way to see this was to walk the term by hand in a remote shell. ADR 0005 gains the event, additive only.

…e in its telemetry Architecture-guardian findings on the asobi#536 fix. The blocking one is mine and it inverted the loop it was meant to repair. `gc_budget_us/1` scaled with the state and had no ceiling, but `next_gc/4` tests the abandon clause first - so once the budget passed `?GC_ABANDON_US` (a state over ~320 MB, inside the range #536 reports) the "this collection overran, back off" branch became unreachable. Everything that would have tripped it abandoned instead, and everything that did not abandon looked cheap, so the interval fell to its minimum on precisely the largest states. The budget is now capped at a quarter of the abandon ceiling, with a test that pins the back-off as reachable at 500 MB. `collect_state/1` now consumes the measured state size rather than reading it. The key is module-owned on a process that may evaluate more than one Luerl state - `init_zone_state/2` boots a throwaway VM to read `spawn_templates`, and stamps its size on the zone process before the zone's own state exists - so a value left behind can be attributed to the wrong state. Taking it makes that at most one tick's worth rather than sticky, and the `undefined` path already degrades to the flat budget. `[asobi, lua, state]` carries `kind` plus the world/match id and coords, from one new key stamped at each bridge's init. Every zone in a world runs the same script, so `script` alone gave a hundred zones one label set and a `last_value` over it is a flapping gauge. It also samples on wall clock now, roughly once a second per bridge, which is the reasoning ADR 0005 already applies to `[asobi, world, tick]` - a per-call counter is a rate that varies with tick rate and multiplies by live zone count. `state_warn_words` and `state_sample_interval_ms` are configuration rather than literals: what counts as a state too large to copy depends on the tick budget, which the loader cannot see. 100 MB is catastrophic at 80 Hz and merely expensive at 2 Hz. Also: `runaway_still_killed/0` passed against a cap a hundred times too wide, so it now runs one callback against two budgets and asserts the verdict follows the configured number. And four comments cited "ADR 0002" for handle_input's exemption, which is the open-registration ADR - one of them had made it into a published `-doc` block. They point at the trust-model guide, which is where the other guides already point. The security note is the one thing #536 genuinely widened: an eval worker now completes instead of being killed at a fixed ceiling, so its peak is `2 x state + max_heap_words`, and zones tick in parallel. Size a node for the sum, not for the budget.

Security review of the asobi#536 fix. The finding is pre-existing - it came in with #426 - but it lives in the code this branch rewrites, and this branch removed the absolute heap cap that used to bound its fallout. `collect/2` anchored `game_state` in `_G` with `luerl:set_table_keys/3`, which honours `__newindex`. `setmetatable(_G, ...)` is explicitly permitted by the trust model, and `unanchor` clears the key every time, so the metamethod fired on every single collection. Both consequences measured: - A `__newindex` that does not return runs script-authored Lua on the bridge gen_server itself, outside `bounded_eval` - no wall-clock budget, no reduction budget, no heap cap. `while true do end` burned 4.7 billion reductions on the zone process and never returned. The zone answers no call again and never terminates, so no supervisor restarts it. - The likelier one, and the reason this is not merely theoretical: an ordinary strict-globals `__newindex` that raises. `set_table_keys/3` catches it, the anchor fails, the collection is skipped - and a skipped collection costs no time, so the adaptive interval reads it as cheap and drives itself to its minimum. 400 ticks: 8417 live tables against 11 for the same script without the metatable, `enabled => true` throughout, no log and no telemetry. #536 re-opened invisibly by one ordinary line of Lua. The anchor is asobi's bookkeeping, not the script's data, so it is written raw. A raw write runs no Lua and cannot fail, which retires the "leave the state alone if the anchor failed" path with it. Both cases are now regression tests: 12 live tables and no hang. `luerl_heap` is internal to luerl, so the dep tightens to `~> 1.5.1` - asobi already depends on `luerl_lib` the same way. Three more from the same review: - The state-size measurement rode on the result message, so it was never sent for a callback killed on heap, time or reductions - the metric went dark on exactly the ticks it exists for, and `gc_budget_us/1` fell back to the flat budget this branch identifies as the pathology. It is now sent separately and first, before the callback can allocate anything, and drained on the kill path rather than left in the bridge's mailbox. - The collector abandoned itself permanently on one slow collection and never re-armed, which fails open: the state then grows unbounded with a single warning to show for it. One overrun is as likely to be a BEAM GC pause as proof the live set is uncollectable, so it retries after five minutes. - `lua_state_large` cleared its latch at the same number it warned at, so a state hovering there logged once a tick. It clears 10% below now. `[asobi, lua, state]` gains `count => 1`, which ADR 0005's own conventions require and every other emitter has. The ADR now also states the key set differs by `kind` and every value may be `undefined` - telemetry detaches a handler that raises, permanently - and that the size excludes Lua strings over 64 bytes, which are refc binaries living off the process heap. That is the right number for what a callback copy costs and the wrong one for what the node is holding.

…given Code review of the asobi#536 fix. The blocker is mine, from the commit before this one. `asobi_world_server:init/1` hands `GameMod:init/1` the game config with `match_id` injected - there is no nested `game_config` key at world level - so `asobi_lua_world:init/1` read its world id out of a map that is always empty and stamped `undefined`. Every world-level `[asobi, lua, state]` series was anonymous, which is the exact failure the ADR and the observability guide describe as worse than having no metric. It reads off `Config` now, the way `make_ctx/1` two functions away already did. The reason nothing caught it is that the identity is stamped at three call sites, each reading a differently-shaped config map, and the only test built its input by hand - so it exercised `bridge_meta/1` and nothing upstream of it. `asobi_lua_bridge_identity_tests` asserts all three against the map their real caller passes; the world case fails without the fix. Smaller ones from the same review: - `hostile_globals_cannot_hang/0` waited 10s inside eunit's 5s per-test timeout, so its kill never ran and the spinning worker burned a scheduler for the rest of the suite. It gets an explicit `{timeout, 30, _}` and a shorter window, plus a `DOWN` clause so a crash reports as a crash rather than as the hang it is not. - `state_words/0` was a public export of the process-dictionary contract with no caller in `src/` - two test lines. It is TEST-only now, definition and all, since the default profile would otherwise carry an unused function. - `live_tables/1` in the tests walked the Luerl state with nested `element/2`. Through the records instead, so a field reorder upstream is a compile error rather than a silently wrong count. That matters more now the dep is pinned to a patch range for exactly this reason. - The re-arm clause left `retry_at` in place when `lua_gc` is configured off, so a bridge with the collector disabled matched it every tick and paid an `application:get_env` forever instead of short-circuiting. - `warn_large_state/3` rebuilt the bookkeeping map every tick to write `false` over `false`, and used `:=` on a key a hand-built map need not have. - `state_sample_interval_ms => 0` means every tick, not off, which is the opposite of what `state_warn_words => 0` means on the line above it in the same table. Said so. - Trimmed the 25-line narrative on `collect/2` down to what only the code can say; the measurements now live in the trust model and the limitations guide.

github-actions · 2026-08-21T08:34:26Z

🟡 Code Coverage — 76.5%

8745 of 11431 lines covered.

🟡 ELP Lint — 192 warnings

210 diagnostics found. See job logs for details.

`rebar3 mutate` does run on asobi - `rebar3_mutate` arrives transitively as a plugin from nova, kura, seki and nova_resilience, so it is in `_build/default/plugins/` without appearing in this repo's `rebar.config`. I had recorded the opposite. A whole-tree run is 11206 mutants and ~40 minutes, which is why CI keeps it off, but `--diff origin/main` scopes it to changed lines and is worth running. Doing that found two things the eunit run could not. The bridge-identity tests lived in their own module, and the plugin pairs a module with `<module>_tests`, so nothing exercised them while mutating `asobi_lua_world` - every `maps:get` in the identity survived statement deletion despite having a test written for it. Same for the new telemetry event, whose only test was in the loader's module. They move to `asobi_lua_world_tests`, `asobi_lua_match_tests` and `asobi_telemetry_tests`, which is where the tooling looks and where a future mutate-diff in CI will too. And it caught a tautology of mine: `size_sample_is_rate_limited/0` passed because `collect_state/1` *consumes* the measurement, so the second and third rounds reported nothing for want of one rather than because the interval suppressed them - the test held whatever the interval did. It refreshes the measurement through a bounded call each round now, and fails if the comparison is inverted. Three real gaps filled while there, one of them the regression test the security review asked for and I had not written: - a callback killed on heap, and one killed down the `kill_and_settle` path, must still report the state size - that path was the whole point of moving the measurement off the result message, - the measurement is consumed rather than read, so it cannot be attributed twice, - an abandoned collector re-arms once its cool-down expires and stays off before that. Not chased: the surviving mutants in the config accessors and their default constants. A test that pins `1000` to `1000` is a test of the literal.

Taure added 4 commits August 21, 2026 08:18

Taure added the gates-passed Architecture, security and code review have run and the owner agreed label Aug 21, 2026

Taure merged commit 0eb74d5 into main Aug 21, 2026
17 checks passed

Taure deleted the fix/lua-heap-budget-relative branch August 21, 2026 11:06

This was referenced Aug 21, 2026

A Lua state has no ceiling; give zones an opt-in one that stops the zone rather than the callback #540

Open

docs(adr): propose the Luerl VM owning the state (ADR 0015) #541

Merged

Navigation Menu

fix(lua): budget the eval heap against the callback, not the state it was handed - #538

fix(lua): budget the eval heap against the callback, not the state it was handed#538
Taure merged 5 commits into
mainfrom
fix/lua-heap-budget-relative

Taure commented Aug 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Taure commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The cap bounded the state, not the callback

The copy is the first problem

Making it visible

A security fix that came out of the review

What this deliberately does not add

Upstream

Verification

Uh oh!

github-actions Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Code Coverage — 76.5%

🟡 ELP Lint — 192 warnings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Taure commented Aug 21, 2026 •

edited

Loading

github-actions Bot commented Aug 21, 2026 •

edited

Loading