lua: a frame that never leaves the VM fails its call, not the client

The load has had an instruction bound since `3b6975fd` — "so a hung load reports
instead of freezing the loading screen" — and the end of the load disarmed it,
"so no handler is killed for being slow". Right about slow, and it leaves a
handler that never returns at all able to freeze the client until the player
kills it, which is what happened to him this morning.

So the bound does not go at the end of the load, it changes shape. Wall time
rather than instructions, because the number has to be obviously beyond anything
legitimate and a count is not: a frame still inside Lua after fifteen seconds is
not slow, it is stuck. `tick` restamps every frame, so no frame is charged the
one before it and a handler reached later in the frame — a click, a fired event —
measures from that mark, which can only make the bound tighter. The raise names
the chunk and the line, so his log says which addon rather than that something
somewhere looped.

It shares the one count hook, which costs 5-13% of pure Lua execution time
(measured: 30M instructions, 235-260 ms bare against 267-274 ms hooked, which
also puts his machine at ~125M VM instructions a second — what sizes the load's
200M at about 1.6 s). Lua is a small part of a play frame, so that is a fraction
of a percent of frame time, and the load was already paying it.
`WOW_LUA_RUNAWAY=<secs>` moves the bound, `0` disarms it.

Two tests: a loop with no exit fails its own call, names its chunk and leaves the
VM usable; and a frame is not charged the wall time that passed before it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn

crates/benilla-app/src/ui_script/lifecycle.rs

@@ -388,6 +388,32 @@ fn report_load(world: &World, start: std::time::Instant, covering: bool, root: &
     );
 }
 
+/// How long one frame may stay inside Lua before the call is failed as non-terminating.
+///
+/// A `/reload` is the longest legitimate stretch of Lua in a session and it runs under the load's
+/// own instruction bound, not this one; during play nothing comes close, and the player's own
+/// heaviest frames are milliseconds. At ~125M VM instructions a second (measured on his machine)
+/// this is some two billion instructions, so a loop reaches it in seconds and a handler never
+/// does. `WOW_LUA_RUNAWAY=<secs>` moves it, `0` disarms it.
+const RUNAWAY_FRAME_SECS: f32 = 15.0;
+
+/// Arm [`UiScript::set_runaway_limit`] for the session, unless the player turned it off. The
+/// client froze on his machine twice in one morning with nothing in the log — a frozen frame
+/// writes no line, so the only way to learn where it stopped is to make the VM refuse to stay
+/// there.
+fn arm_runaway_limit(script: &UiScript) {
+    let secs = std::env::var("WOW_LUA_RUNAWAY")
+        .ok()
+        .and_then(|v| v.parse::<f32>().ok())
+        .unwrap_or(RUNAWAY_FRAME_SECS);
+    if secs <= 0.0 {
+        script.clear_instruction_budget();
+        info!("ui_script: runaway-frame bound disarmed (WOW_LUA_RUNAWAY=0)");
+        return;
+    }
+    script.set_runaway_limit(std::time::Duration::from_secs_f32(secs));
+}
+
 /// Seat the VM's screen size and text measurer for the load, under the current window and
 /// `uiScale`: the load-edge half of [`super::extract::tick_script`]'s per-frame pair. A fresh VM
 /// answers 1024×768 until then, and a value computed once at load keeps it: stock
@@ -567,8 +593,9 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) {
     // and never again: `[0xce8820]` has one writer, and a `/reloadui` does not re-run it, where
     // this edge re-reads it on every reload.
     world.insert_resource(crate::combat_text::read_damage_text_font(&script));
-    // The load is over: disarm its instruction bound, so no handler is killed for being slow.
-    script.clear_instruction_budget();
+    // The load is over: its instruction bound goes, so no handler is killed for being slow, and
+    // a wall-clock bound takes over, so one that never returns at all is killed anyway.
+    arm_runaway_limit(&script);
     world.insert_non_send_resource(script);
     world.insert_resource(AddOnIdentity(identity));
     // Arm the world latch, the reference's `0x490168 call 0x4908c0`: a reload creates no entity

crates/benilla-ui/src/script/mod.rs

@@ -498,9 +498,21 @@ pub struct UiScript {
     lua: Lua,
     /// VM instructions counted while a budget is installed ([`UiScript::set_instruction_budget`]).
     instructions: std::sync::Arc<std::sync::atomic::AtomicU64>,
+    /// When this frame's Lua began, for [`UiScript::set_runaway_limit`]; nanoseconds on
+    /// [`mono_nanos`]'s clock, shared with the hook closure.
+    vm_entry: std::sync::Arc<std::sync::atomic::AtomicU64>,
     session: u64,
 }
 
+/// Monotonic nanoseconds since the first call, which is a VM's birth: a plain `u64` an atomic can
+/// hold and a hook can read without allocating or locking.
+pub(crate) fn mono_nanos() -> u64 {
+    static BASE: std::sync::OnceLock<std::time::Instant> = std::sync::OnceLock::new();
+    let base = BASE.get_or_init(std::time::Instant::now);
+    // A u64 of nanoseconds runs for 584 years.
+    base.elapsed().as_nanos() as u64
+}
+
 /// Hands out [`UiScript::session`] ids: process-global and monotone, so an id is never reused.
 static NEXT_SESSION: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
 
@@ -636,6 +648,7 @@ impl UiScript {
         let s = UiScript {
             lua,
             instructions: std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)),
+            vm_entry: std::sync::Arc::new(std::sync::atomic::AtomicU64::new(mono_nanos())),
             session: NEXT_SESSION.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
         };
         Ok(s)
@@ -690,6 +703,65 @@ impl UiScript {
         self.lua.remove_hook();
     }
 
+    /// Bound how long **one frame's** Lua may run in wall time: past `limit` the hook raises a Lua
+    /// error naming the line it was on, so a handler that never returns fails its own call and the
+    /// client keeps running instead of freezing until the player kills it.
+    ///
+    /// This is the play-time counterpart of [`Self::set_instruction_budget`], which covers only
+    /// the load. Wall time rather than instructions because the number has to be obviously beyond
+    /// anything legitimate and a count is not: a frame still inside Lua after seconds is not slow,
+    /// it is stuck. [`Self::tick`] restamps every frame, so no frame is charged the one before it.
+    ///
+    /// It shares the one count hook, which costs **5-13% of pure Lua execution time** (measured,
+    /// 30M instructions on the player's machine: 235-260 ms bare, 267-274 ms hooked) — a fraction
+    /// of a percent of a frame during play, where Lua is a fraction of the frame. The load already
+    /// paid it.
+    pub fn set_runaway_limit(&self, limit: std::time::Duration) {
+        let entry = self.vm_entry.clone();
+        entry.store(mono_nanos(), std::sync::atomic::Ordering::Relaxed);
+        let limit_ns = u64::try_from(limit.as_nanos()).unwrap_or(u64::MAX);
+        let _ = self.lua.set_hook(
+            mlua::HookTriggers {
+                every_nth_instruction: Some(INSTRUCTION_HOOK_STEP),
+                ..Default::default()
+            },
+            move |_, debug| {
+                let started = entry.load(std::sync::atomic::Ordering::Relaxed);
+                let now = mono_nanos();
+                let spent = now.saturating_sub(started);
+                if spent <= limit_ns {
+                    return Ok(mlua::VmState::Continue);
+                }
+                // Restamp before raising: the error unwinds to whatever pcall caught the call, and
+                // the rest of the frame — and every frame after it — deserves a whole budget.
+                entry.store(now, std::sync::atomic::Ordering::Relaxed);
+                let src = debug.source();
+                Err(mlua::Error::runtime(format!(
+                    "benilla: {:.1} s inside Lua in one frame, at {}:{} — treating it as a \
+                     non-terminating loop and failing the call, so the client keeps running",
+                    spent as f64 / 1e9,
+                    src.short_src.as_deref().unwrap_or("?"),
+                    debug.current_line().unwrap_or(0),
+                )))
+            },
+        );
+    }
+
+    /// Restamp this frame's Lua clock for [`Self::set_runaway_limit`]. [`Self::tick`] calls it, so
+    /// a host that drives the VM by hand (a test, the harness) can arm a limit and still get a
+    /// fresh budget per frame.
+    pub fn mark_vm_entry(&self) {
+        self.vm_entry
+            .store(mono_nanos(), std::sync::atomic::Ordering::Relaxed);
+    }
+
+    /// How long this frame has been inside Lua, for a watchdog that wants to say whether a stuck
+    /// frame is stuck in the VM. Meaningless unless a limit is armed.
+    pub fn frame_lua_secs(&self) -> f32 {
+        let started = self.vm_entry.load(std::sync::atomic::Ordering::Relaxed);
+        mono_nanos().saturating_sub(started) as f32 / 1e9
+    }
+
     /// VM instructions since the last [`Self::set_instruction_budget`], to the resolution of
     /// [`INSTRUCTION_HOOK_STEP`]; zero when no budget was ever set.
     pub fn instructions_used(&self) -> u64 {

crates/benilla-ui/src/script/tests/mod.rs

@@ -38,6 +38,7 @@ mod reference_surface;
 mod region_map;
 mod regions;
 mod resize_bounds;
+mod runaway_frame;
 mod scrollframe;
 mod setparent;
 mod simplehtml;

crates/benilla-ui/src/script/tests/runaway_frame.rs

@@ -0,0 +1,57 @@
+//! The runaway-frame bound: a call that never returns fails itself instead of freezing the client,
+//! and every frame gets the whole bound.
+
+use std::time::Duration;
+
+use crate::script::UiScript;
+
+/// Short enough to keep the test instant, and far longer than the hook's 1M-instruction step.
+const LIMIT: Duration = Duration::from_millis(80);
+
+#[test]
+fn a_loop_that_never_ends_fails_its_own_call_and_names_where_it_was() {
+    let s = UiScript::new().expect("a VM");
+    s.set_runaway_limit(LIMIT);
+
+    let err = s
+        .lua()
+        .load("while true do end")
+        .set_name("Interface\\AddOns\\Stuck\\Stuck.lua")
+        .exec()
+        .expect_err("a loop with no exit is stopped");
+    let text = err.to_string();
+    assert!(
+        text.contains("non-terminating loop"),
+        "the raise says what it decided: {text}"
+    );
+    // The player's log has to name the file, or the next freeze is another bisect.
+    assert!(
+        text.contains("Stuck.lua"),
+        "the raise names the chunk it stopped in: {text}"
+    );
+
+    // The VM is still usable: the bound failed one call, it did not poison the state.
+    s.lua()
+        .load("benilla_runaway_test = 1 + 1")
+        .exec()
+        .expect("the VM survives its own refusal");
+}
+
+#[test]
+fn a_frame_is_not_charged_the_one_before_it() {
+    let s = UiScript::new().expect("a VM");
+    s.set_runaway_limit(LIMIT);
+    // Spend the whole bound without ever being inside Lua for it: wall time passes between
+    // frames, which is exactly what the mark exists to forgive.
+    std::thread::sleep(LIMIT * 2);
+    s.mark_vm_entry();
+    s.lua()
+        .load("local n = 0 for i = 1, 100 do n = n + i end")
+        .exec()
+        .expect("a short call in a fresh frame runs");
+
+    // Without the mark the same sleep would have spent it, and the next long-enough call raises.
+    std::thread::sleep(LIMIT * 2);
+    let err = s.lua().load("while true do end").exec();
+    assert!(err.is_err(), "an unmarked frame still runs out");
+}

crates/benilla-ui/src/script/tick.rs

@@ -98,6 +98,10 @@ impl super::UiScript {
     /// `OnUpdate(self, elapsed)` on every visible frame that has one (`0x704f10`), then the
     /// engine's own fades, model panes and hover.
     pub fn tick(&mut self, elapsed: f32) {
+        // This frame's Lua starts here, for [`Self::set_runaway_limit`]: a frame is not charged
+        // the one before it, and a handler reached later in the frame (a click, a fired event)
+        // measures from this mark, which can only make the bound tighter, never looser.
+        self.mark_vm_entry();
         // `WOW_UI_TICK=<secs>`: the phase split ([`super::tick_prof`]). Inert when off, and every
         // phase below marks, so the row sums to the `[ui-cost] tick=` figure.
         let mut split = Split::open(&self.lua);