grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lua: a size this library derived is a cache, and a # append invalidates it

Browse Source 
This is what froze his client. The bound named `Cinematic/Camera.lua:2597`:

    while turnStarts[1] and now - turnStarts[1] > WINDOW do
        table.remove(turnStarts, 1)
    end

`remove` drains the table and remembers a size of 0 — faithfully, that is
`luaL_setn`. The addon then appends with `turnStarts[#turnStarts + 1] = now`,
which is a raw set this layer never sees, so slot 1 is full while the
remembered size is still 0. `remove` reads 0, takes its "nothing to remove"
path, changes nothing, and the caller spins forever. 15 s of Lua in every
frame, for the whole session, in a loop that in the reference cannot fail.

It cannot fail there because 5.0 has no `#` operator: every append goes through
`insert` or `getn`, so a remembered size is always the truth. The collision is
a later-era addon's `#` meeting 5.0's bookkeeping, and it also loses data the
quiet way round — `insert` after a `#` append writes *over* the appended slot
instead of after it, which is a test here now too.

So the two kinds of size are kept apart. One this library **derived** from an
`insert` or a `remove` is a cache: if `t[n + 1]` is not nil the table has grown
since, and the count wins. One an addon **declared** with `table.setn` is
honoured exactly as before, because `setn(t, 0)` as a logical wipe over full
slots is the entire point of it — AceLibrary and Mik's capture tables both turn
on that, and both still pass. An explicit numeric `t.n` field is a declaration
too, and is untouched.

Measured: the corpus roster is byte-identical (87 ok / 1 fail, the failure the
pre-existing AdvancedTradeSkillWindow2), and the drain loop that spun 44 times
in his log now ends in one removal a round.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
grom
2026-11-02 08:03:09 -07:00
co-authored by Claude Opus 5
parent d1edd74cce
commit c6b19c7926
1 changed files with 171 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-ui/src/script/lua50/table_size.rs
+171 -41
View File
@@ -9,12 +9,90 @@
//!
//! The side store must stay invisible: a stray `n` key breaks mixin walks such as `AceOO-2.0`'s,
//! which error on any field the target already has.
//!
//! ## The one place 5.0's remembered size and 5.1's `#` collide
//!
//! 5.0 has no `#` operator, so in the reference nothing can grow a table behind the library's
//! back and a remembered size is always the truth. A later-era addon appends with `t[#t + 1] = v`,
//! which is a raw set this layer never sees — so after `remove` has stored a size of 0, the next
//! `#` append writes slot 1 and `remove` reads 0, refuses, and the caller spins:
//!
//! ```lua
//! while turnStarts[1] and now - turnStarts[1] > WINDOW do table.remove(turnStarts, 1) end
//! ```
//!
//! That is `Cinematic/Camera.lua:2597`, and it froze the client twice on 2026-11-02 — 15 s of Lua
//! in every frame, for the whole session. The same mismatch silently *loses* data the other way:
//! `insert` after a `#` append writes over the appended slot instead of after it.
//!
//! So a size this library **derived** is treated as a cache: if `t[n + 1]` is not nil the table has
//! grown since, and the count wins. A size an addon **declared** with `table.setn` is still
//! honoured exactly as 5.0 honours it — that is the whole point of `setn(t, 0)` as a logical wipe
//! (`AceLibrary`, Mik's capture tables), and it is also why the two cases cannot share one slot.
//! An explicit numeric `t.n` field is a declaration too, and stays untouched.
use mlua::{Function, Lua, MultiValue, Table, Value};
/// The side store's registry name; 5.0 uses slot 2, and either way no addon can reach it.
const SIZES: &str = "benilla.lua50.sizes";
/// Beside it, which of those sizes an addon **declared** with `table.setn` rather than this
/// library deriving it from an `insert` or a `remove`. Only a derived one is a cache; see the
/// module doc.
const DECLARED: &str = "benilla.lua50.sizes.declared";
/// The remembered size, and whether the addon declared it. Cloned into each closure below, as the
/// one table was before: two weak-keyed tables, neither reachable from Lua.
#[derive(Clone)]
struct Sizes {
store: Table,
declared: Table,
}
impl Sizes {
/// `luaL_getn` (`0x6f5050`); its count to the first nil is not the `#` border on a holed
/// table. A size this library derived is dropped when the table has grown past it.
fn get(&self, t: &Table) -> mlua::Result<i64> {
if let Some(n) = as_size(&t.raw_get::<Value>("n")?) {
return Ok(n);
}
if let Some(n) = as_size(&self.store.raw_get::<Value>(t.clone())?) {
let declared = self.declared.raw_get::<Value>(t.clone())?;
let grown = !matches!(t.raw_get::<Value>(n + 1)?, Value::Nil);
if matches!(declared, Value::Boolean(true)) || !grown {
return Ok(n);
}
}
let mut i = 1i64;
while !matches!(t.raw_get::<Value>(i)?, Value::Nil) {
i += 1;
}
Ok(i - 1)
}
/// `luaL_setn` (`0x6f4ea0`): into `t.n` when the table has one, else the side store. `insert`
/// and `remove` call this, so the size they write is derived and the declaration is spent.
fn set(&self, t: &Table, n: i64) -> mlua::Result<()> {
self.declared.raw_set(t.clone(), Value::Nil)?;
self.write(t, n)
}
/// The same write from `table.setn`: the addon is stating the size, so it is honoured even
/// where the table says otherwise.
fn declare(&self, t: &Table, n: i64) -> mlua::Result<()> {
self.declared.raw_set(t.clone(), true)?;
self.write(t, n)
}
fn write(&self, t: &Table, n: i64) -> mlua::Result<()> {
if as_size(&t.raw_get::<Value>("n")?).is_some() {
t.raw_set("n", n)
} else {
self.store.raw_set(t.clone(), n)
}
}
}
/// `checkint` (`0x6f4f80`): a stored size counts only if numeric and non-negative. The reference
/// also takes a numeric string through `lua_tonumber`; this does not.
fn as_size(v: &Value) -> Option<i64> {
@@ -34,55 +112,38 @@ fn as_size(v: &Value) -> Option<i64> {
/// is beyond the VM's instruction budget: a hung client.
const MAX_SHIFT: i64 = 1_000_000;
/// `luaL_getn` (`0x6f5050`); its count to the first nil is not the `#` border on a holed table.
fn get_n(sizes: &Table, t: &Table) -> mlua::Result<i64> {
if let Some(n) = as_size(&t.raw_get::<Value>("n")?) {
return Ok(n);
}
if let Some(n) = as_size(&sizes.raw_get::<Value>(t.clone())?) {
return Ok(n);
}
let mut i = 1i64;
while !matches!(t.raw_get::<Value>(i)?, Value::Nil) {
i += 1;
}
Ok(i - 1)
}
/// `luaL_setn` (`0x6f4ea0`): into `t.n` when the table has one, else the side store.
fn set_n(sizes: &Table, t: &Table, n: i64) -> mlua::Result<()> {
if as_size(&t.raw_get::<Value>("n")?).is_some() {
t.raw_set("n", n)
} else {
sizes.raw_set(t.clone(), n)
}
}
/// Install the `n`-based table library over mlua's 5.1 one. `concat` and `sort` wrap the C bodies
/// for their error messages and algorithm (Rust's `sort_by` panics on an inconsistent comparator).
pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
let g = lua.globals();
let store = lua.create_table()?;
let meta = lua.create_table()?;
// Weak keys, as 5.0's `getsizes` builds it: a table nobody else holds stays collectable.
meta.set("__mode", "k")?;
store.set_metatable(Some(meta))?;
// Held by the registry for the VM's life; each closure below captures it.
lua.set_named_registry_value(SIZES, store.clone())?;
let sizes = store;
let weak = |lua: &Lua| -> mlua::Result<Table> {
let t = lua.create_table()?;
let meta = lua.create_table()?;
meta.set("__mode", "k")?;
t.set_metatable(Some(meta))?;
Ok(t)
};
let sizes = Sizes {
store: weak(lua)?,
declared: weak(lua)?,
};
// Held by the registry for the VM's life; each closure below captures its own clone.
lua.set_named_registry_value(SIZES, sizes.store.clone())?;
lua.set_named_registry_value(DECLARED, sizes.declared.clone())?;
let t: Table = g.get("table")?;
let getn_sizes = sizes.clone();
t.set(
"getn",
lua.create_function(move |_, t: Table| get_n(&getn_sizes, &t))?,
lua.create_function(move |_, t: Table| getn_sizes.get(&t))?,
)?;
// `table.setn` (`0x7fb670`): no results.
let setn_sizes = sizes.clone();
t.set(
"setn",
lua.create_function(move |_, (t, n): (Table, i64)| set_n(&setn_sizes, &t, n))?,
lua.create_function(move |_, (t, n): (Table, i64)| setn_sizes.declare(&t, n))?,
)?;
// `table.insert` (`0x7fb6a0`): `n = getn(t) + 1`, raised to a later position; `setn` first,
@@ -98,7 +159,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
"bad argument #1 to 'insert' (table expected)",
));
};
let mut n = get_n(&insert_sizes, &t)? + 1;
let mut n = insert_sizes.get(&t)? + 1;
// The 2-vs-3 argument split is `lua_gettop`, so an explicit trailing nil counts:
// `insert(t, 4, nil)` writes nil at 4, it does not append the 4.
let (pos, v) = if argc <= 2 {
@@ -122,7 +183,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
if n.saturating_sub(pos) > MAX_SHIFT {
return Err(mlua::Error::runtime("table too big to insert into"));
}
set_n(&insert_sizes, &t, n)?;
insert_sizes.set(&t, n)?;
while n > pos {
let above: Value = t.raw_get(n - 1)?;
t.raw_set(n, above)?;
@@ -138,7 +199,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
t.set(
"remove",
lua.create_function(move |_, (t, pos): (Table, Option<i64>)| {
let n = get_n(&remove_sizes, &t)?;
let n = remove_sizes.get(&t)?;
let pos = pos.unwrap_or(n);
if n <= 0 {
return Ok(MultiValue::new());
@@ -146,7 +207,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
if n.saturating_sub(pos) > MAX_SHIFT {
return Err(mlua::Error::runtime("table too big to remove from"));
}
set_n(&remove_sizes, &t, n - 1)?;
remove_sizes.set(&t, n - 1)?;
let taken: Value = t.raw_get(pos)?;
for i in pos..n {
let next: Value = t.raw_get(i + 1)?;
@@ -162,7 +223,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
t.set(
"foreachi",
lua.create_function(move |_, (t, f): (Table, Function)| {
let n = get_n(&foreachi_sizes, &t)?;
let n = foreachi_sizes.get(&t)?;
for i in 1..=n {
let v: Value = t.raw_get(i)?;
let r: Value = f.call((i, v))?;
@@ -183,7 +244,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
move |_, (t, sep, i, j): (Table, Option<String>, Option<i64>, Option<i64>)| {
let j = match j {
Some(j) => j,
None => get_n(&concat_sizes, &t)?,
None => concat_sizes.get(&t)?,
};
concat.call::<Value>((t, sep.unwrap_or_default(), i.unwrap_or(1), j))
},
@@ -198,7 +259,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
t.set(
"sort",
lua.create_function(move |_, (t, cmp): (Table, Option<Function>)| {
let n = get_n(&sort_sizes, &t)?;
let n = sort_sizes.get(&t)?;
let border = i64::try_from(t.raw_len()).unwrap_or(i64::MAX);
if n >= border {
return sort.call::<()>((t, cmp));
@@ -226,7 +287,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
let i = i.unwrap_or(1);
let j = match j {
Some(j) => j,
None => get_n(&sizes, &t)?,
None => sizes.get(&t)?,
};
// The reference's `luaL_checkstack(n, "table too big to unpack")`, at `MAX_SHIFT`.
if j.saturating_sub(i) >= MAX_SHIFT {
@@ -247,6 +308,75 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
mod tests {
use crate::script::UiScript;
/// The shape that froze the client: `remove` drains a table to a remembered 0, a later-era
/// addon appends with `#`, and `remove` is asked again. `Cinematic/Camera.lua:2597` is this
/// loop, and it spent 15 s of every frame in it for a whole session.
#[test]
fn a_hash_append_after_a_drain_is_removable_again() {
let s = UiScript::new().unwrap();
assert_eq!(
s.eval::<i64>(
r#"
local t, spins = {}, 0
for round = 1, 3 do
t[#t + 1] = round
while t[1] and spins < 50 do table.remove(t, 1) spins = spins + 1 end
end
return spins
"#
)
.unwrap(),
3,
"one removal per round; 50 is the cap, and the cap means the drain never ends"
);
}
/// The same mismatch the other way, which loses data instead of hanging: `insert` must land
/// after a slot the `#` append wrote, not on top of it.
#[test]
fn insert_after_a_hash_append_does_not_overwrite_it() {
let s = UiScript::new().unwrap();
assert_eq!(
s.eval::<String>(
r#"
local t = {}
table.insert(t, "a")
t[#t + 1] = "b"
table.insert(t, "c")
return table.concat(t, ",")
"#
)
.unwrap(),
"a,b,c",
"a derived size that the table has outgrown is stale, so insert must count instead"
);
}
/// And the declaration it must not break: `setn(t, 0)` is 5.0's logical wipe, honoured even
/// though every slot it covers is still full. Only a size this library *derived* is a cache.
#[test]
fn an_explicit_setn_still_wins_over_the_tables_own_contents() {
let s = UiScript::new().unwrap();
assert_eq!(
s.eval::<i64>("local t = {1,2,3} table.setn(t, 0) return table.getn(t)")
.unwrap(),
0,
"the addon declared 0 with slots 1..3 full; that is the whole point of setn"
);
assert_eq!(
s.eval::<i64>("local t = {1,2,3} table.setn(t, 0) table.insert(t, 9) return t[1]")
.unwrap(),
9,
"5.0's insert writes getn+1, which after setn(t, 0) is index 1"
);
assert_eq!(
s.eval::<i64>("local t = {1,2,3} table.setn(t, 7) return table.getn(t)")
.unwrap(),
7,
"a declaration past the end is honoured too"
);
}
/// Mik's Scrolling Battle Text 4.43's capture tables: wipe, `setn(t, 0)`, then insert.
#[test]
fn a_wipe_then_setn_zero_then_insert_is_countable_again() {