Skip to content

Coroutine stack truncated between AsyncThread polls corrupts hook-yielded coroutines #723

Description

@johalun

Summary

A coroutine suspended by a debug hook returning VmState::Yield and driven as an AsyncThread can have its stack truncated below its own live registers. The interrupted function is later resumed with its locals set to nil.

Detail

Both AsyncThread poll paths wrap the resume in StackGuard::with_top(thread_state, 0), resetting the coroutine's stack on the way out. Whether that is safe depends on where the coroutine is parked:

  • An ordinary yield — coroutine.yield, or the async poll protocol — is a call. The innermost frame is coroutine.yield's C frame, which sits above every Lua frame's registers, so truncating cannot lose live data.
  • A count/line hook returning VmState::Yield is an interruption. There is no yield call: the innermost frame is the interrupted Lua function itself, and its live registers sit above where lua_settop(L, 0) leaves top.

Lua marks a coroutine's stack only up to top, and nils every slot above it during the atomic phase (traversethread in lgc.c). So once any allocation elsewhere on the same lua_State drives a GC cycle far enough, the suspended function loses its locals. Resuming also passes the frame's register count as the resume argument count, via resumable_nargs.

Symptoms depend on which frame is reached:

  • Usually attempt to perform arithmetic on a nil value (local 'x') raised against the user's own script.
  • When the frame reached is the internal __mlua_async_poll chunk, its future local is cleared and poll_future panics on a null userdata pointer. Because that is raised while unwinding a StackGuard, it becomes a non-unwinding panic that aborts the process:
mlua internal error: userdata pointer is null (this is a bug, please file an issue)
mlua internal error: N too many stack values popped (this is a bug, please file an issue)
panic in a destructor during cleanup
thread caused non-unwinding panic. aborting.

Reproducing

Two AsyncThreads on one Lua, driven by a single-threaded runtime: one CPU-bound and instruction-sliced by a yielding hook, one calling an async function in a loop. Note the assertion is on the returned value — the failure is a corrupted result, not only a crash.

use mlua::{Function, HookTriggers, Lua, VmState};

fn make_lua() -> Lua {
    let lua = Lua::new();
    let sleep = lua
        .create_async_function(|_, ms: u64| async move {
            tokio::time::sleep(std::time::Duration::from_millis(ms)).await;
            Ok(())
        })
        .unwrap();
    lua.globals().set("sleep", sleep).unwrap();
    lua.load(
        r#"
        function busy()
            local x = 0
            for i = 1, 40000000 do x = x + 1 end
            return x
        end
        function awaits()
            local n = 0
            for i = 1, 300 do sleep(1) n = n + 1 end
            return n
        end
        "#,
    )
    .exec()
    .unwrap();
    lua
}

/// `sliced`: install a count hook that yields, i.e. preempt this coroutine.
fn spawn(lua: &Lua, name: &str, sliced: bool) -> mlua::thread::AsyncThread<i64> {
    let f: Function = lua.globals().get(name).unwrap();
    let thread = lua.create_thread(f).unwrap();
    if sliced {
        thread
            .set_hook(HookTriggers::new().every_nth_instruction(10_000), |_, _| {
                Ok(VmState::Yield)
            })
            .unwrap();
    }
    thread.into_async::<i64>(()).unwrap()
}

#[tokio::test]
async fn sliced_coroutine_keeps_its_locals() {
    let lua = make_lua();
    let (busy, awaits) = tokio::join!(spawn(&lua, "busy", true), spawn(&lua, "awaits", false));
    assert_eq!(busy.unwrap(), 40_000_000);
    assert_eq!(awaits.unwrap(), 300);
}

Fails on every run for me with mlua 0.12.0, features lua54, vendored, macros, async.

It only fails once the overlap is long enough to reach an atomic GC phase, which is what makes it look intermittent — shortening the loop to a few million iterations passes. Slicing a coroutine that runs alone also mostly survives, because it allocates too little to advance the collector.

Also reproduces on 0.12.0-rc.1, so it is not a recent regression.

Use case

We use instruction-count hooks as a preemption mechanism: user-supplied Lua runs on a shared Lua per worker thread, one coroutine per job, and the hook yields periodically so that a heavy script cannot monopolise the worker. Those same scripts call async functions for I/O. That combination — several AsyncThreads on one state, at least one sliced, at least one awaiting — is the failing shape.

Possible direction

Distinguishing the two cases looks tractable: for a suspended thread, checking whether the level-0 frame is a Lua function separates a hook yield from an ordinary one. From there the truncation can be skipped, no resume arguments passed, and the Poll::Pending marker probe skipped. Ordinary yields keep today's cleanup, written explicitly rather than by the guard — because whether truncation is safe depends on how the thread parks on the way out of a poll, not on how it arrived.

I have a patch along these lines that fixes the repro above and keeps the existing test suite green. Happy to open a PR if the approach seems reasonable — I wanted to check the framing first, in particular:

  1. Are hook yields considered supported for AsyncThread? process_status implements VmState::Yield for count/line hooks under 5.3+, so I assumed yes, but I would rather ask than assume.
  2. How would you want the synchronous Thread::resume / resume_error paths handled? They use the same guard and additionally push resume arguments on top of the live frame, so they look affected too. I have not tested them, and my patch does not touch them.

Note

Discovered, triaged and authored by Claude Fable

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions