mlua-rs / mlua

High level Lua 5.4/5.3/5.2/5.1 (including LuaJIT) and Roblox Luau bindings to Rust with async/await support
Other
1.73k stars 138 forks source link

SIGSEGV in version `0.9.0` when using `Variadic<T>` #311

Closed filiptibell closed 1 year ago

filiptibell commented 1 year ago

One of our tests is currently causing SIGSEGV using mlua version 0.9.0 (full release) when using Variadic<UserDataRef<T>>, even though we are not using unsafe anywhere in our codebase:

https://github.com/filiptibell/lune/blob/83db30496d729a70038501ffae7317e549ef31f8/tests/roblox/datatypes/CFrame.luau#L102-L112

This is the relevant piece of source code in our repo:

https://github.com/filiptibell/lune/blob/83db30496d729a70038501ffae7317e549ef31f8/src/roblox/datatypes/types/cframe.rs#L213-L225

And a minimal implementation of our userdata type, which is a wrapper around glam::Mat4

#[derive(Debug, Clone, Copy, PartialEq)]
pub struct CFrame(pub Mat4);

impl LuaUserData for CFrame {
    fn add_methods<'lua, M: LuaUserDataMethods<'lua, Self>>(methods: &mut M) {
        methods.add_method(
            "ToWorldSpace",
            |_, this, rhs: Variadic<LuaUserDataRef<CFrame>>| {
                Ok(Variadic::from_iter(rhs.into_iter().map(|cf| *this * *cf)))
            },
        );
    }
}

impl std::ops::Mul for CFrame {
    type Output = Self;
    fn mul(self, rhs: Self) -> Self::Output {
        CFrame(self.0 * rhs.0)
    }
}
khvzak commented 1 year ago

I reproduced the issue, thanks.

It's an edge case related to using wrong "self" (userdata) index after extracting arguments from the stack and invalidating relative indexes (works wrong only for Variadic/MultiValue types). The new mlua has many performance improvements and one of them is extracting function arguments directly from Lua stack without creating intermediate data structures and exchanging values between main thread and "reference" thread.

The SIGSEGV you've seen it's a c++ assertion that checks correctness of Lua API usage.

filiptibell commented 1 year ago

Thank you for fixing this so quickly!