fix(ui): WOW_UI_VERBS must not wrap setfenv — it unloaded a third of the interface
A wrapper puts its own frame between the caller and the callee, which silently changes the meaning of every stack-level-relative call. `verb_prof::install` wraps every function in `_G`, and `_G` at that point holds Lua's own stdlib as well as our bindings — so it wrapped `setfenv`. pfUI's `api/api.lua:4` is `setfenv(1, pfUI:GetEnvironment())`, and level 1 from inside a wrapper is the wrapper, a C function, so Lua raises ``setfenv' cannot change environment of given object`. One armed session logged **91** of those and lost pfUI, WhatsTraining_Turtle, AI_VoiceOver, MasterTradeSkills, pfUI-addonskinner, pfUI-locplus and PizzaSlices: an instrument that unloaded a third of the interface it was there to measure, and whose numbers were therefore of a client without them. `STACK_RELATIVE` names the four `wrap_table` must leave alone. `setfenv` and `getfenv` take a level; `error(msg, level)` positions its message by one; and `debugstack([start])` is the reference's traceback by level, which `FuBarPlugin-2.0.lua:752` asks for as `debugstack(6, 1, 0)` and `AceDB-2.0.lua:742` reads a specific line of. `pcall`/`xpcall` stay wrapped: no level argument, and an armed session shows them working and worth seeing. The test wraps `_G` and then runs pfUI's line, so it is the bug rather than a restatement of the list; deleting the skip reproduces grom's exact error. The module doc also claimed `_G` "holds our bindings and nothing else" at install time, which is what made this easy to miss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -8,8 +8,9 @@
|
||||
//! (`CreateFrame`, `SetPoint`, `SetTexture`, `Show`), and nothing charged it to them.
|
||||
//!
|
||||
//! **How it attaches.** Armed, [`install`] walks `_G` and every widget method table at the end of
|
||||
//! [`UiScript::new`](super::UiScript::new) — before any FrameXML or addon runs, so everything it
|
||||
//! finds is ours — and replaces each function with a closure that times the call. A verb that
|
||||
//! [`UiScript::new`](super::UiScript::new) — before any FrameXML or addon runs, so every function
|
||||
//! it finds is one of ours or one of Lua's own, never an addon's — and replaces each with a closure
|
||||
//! that times the call. Lua's own are in there too, which is why [`STACK_RELATIVE`] exists. A verb that
|
||||
//! calls back into Lua (a handler fired from inside `Show`) has that nested time inside its own,
|
||||
//! as the handler profiler's *total* column does; the two instruments read against each other.
|
||||
//!
|
||||
@@ -234,6 +235,23 @@ pub fn reset() {
|
||||
*ROWS.lock().unwrap_or_else(|p| p.into_inner()) = None;
|
||||
}
|
||||
|
||||
/// Functions [`wrap_table`] must leave alone: each reads the **call stack by level**, and a
|
||||
/// wrapper puts its own frame between the caller and the callee, so the level means something
|
||||
/// else.
|
||||
///
|
||||
/// `setfenv` is the one that bit, in grom's 2026-10-19 session. pfUI's `api/api.lua:4` is
|
||||
/// `setfenv(1, pfUI:GetEnvironment())`; level 1 from inside a wrapper is the wrapper, which is a C
|
||||
/// function, so Lua raises ``setfenv' cannot change environment of given object`. That one armed
|
||||
/// run logged **91** of those and took pfUI, AI_VoiceOver, WhatsTraining_Turtle, MasterTradeSkills
|
||||
/// and others down with it — the instrument unloaded a third of the interface it was there to
|
||||
/// measure, and the session's numbers were of a client without them.
|
||||
///
|
||||
/// `error(msg, level)` positions the message by level, and `debugstack([start])` is the reference's
|
||||
/// traceback by level — `FuBarPlugin-2.0.lua:752` asks for `debugstack(6, 1, 0)` and
|
||||
/// `AceDB-2.0.lua:742` reads a specific line of it. `pcall`/`xpcall` stay wrapped: they take no
|
||||
/// level, and one armed session shows them working and worth seeing (pfUI's feature probes).
|
||||
const STACK_RELATIVE: &[&str] = &["setfenv", "getfenv", "error", "debugstack"];
|
||||
|
||||
/// Wrap one table's function values in place, naming each `<prefix><key>`.
|
||||
fn wrap_table(lua: &Lua, table: &Table, prefix: &str) -> mlua::Result<usize> {
|
||||
let mut wrapped = 0;
|
||||
@@ -247,6 +265,9 @@ fn wrap_table(lua: &Lua, table: &Table, prefix: &str) -> mlua::Result<usize> {
|
||||
})
|
||||
.collect();
|
||||
for key in keys {
|
||||
if STACK_RELATIVE.contains(&key.as_str()) {
|
||||
continue;
|
||||
}
|
||||
let Ok(Value::Function(orig)) = table.raw_get::<Value>(key.as_str()) else {
|
||||
continue;
|
||||
};
|
||||
@@ -318,4 +339,28 @@ mod tests {
|
||||
rows[0].micros
|
||||
);
|
||||
}
|
||||
|
||||
/// The 2026-10-19 regression, as a falsifier. Wrapping `_G` wholesale took `setfenv` with it,
|
||||
/// and `setfenv(1, env)` from a chunk then raised ``setfenv' cannot change environment of
|
||||
/// given object` — level 1 was the wrapper, a C function. pfUI's `api/api.lua:4` is exactly
|
||||
/// this line, which is why one armed session cost 91 load errors.
|
||||
#[test]
|
||||
fn wrapping_the_globals_spares_the_stack_relative_ones() {
|
||||
let lua = mlua::Lua::new();
|
||||
super::wrap_table(&lua, &lua.globals(), "").expect("wrap _G");
|
||||
// The globals are bound to locals first: after `setfenv` the chunk's own lookups go
|
||||
// through the new environment, where they are not.
|
||||
let answered: bool = lua
|
||||
.load(
|
||||
r#"
|
||||
local setfenv, getfenv = setfenv, getfenv
|
||||
local env = { ok = true }
|
||||
setfenv(1, env)
|
||||
return ok and getfenv(1) == env
|
||||
"#,
|
||||
)
|
||||
.eval()
|
||||
.expect("setfenv(1, …) must still reach the calling chunk");
|
||||
assert!(answered, "the chunk got someone else's environment");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user