rust-lua53 icon indicating copy to clipboard operation
rust-lua53 copied to clipboard

Thread can outilve the main state

Open therustmonk opened this issue 7 years ago • 2 comments

This code takes segfault:

extern crate lua;
use lua::*;
fn main() {
    let mut thread = {
        let mut state = State::new();
        state.new_thread()
    };
    thread.do_string(r#"print("Hello, segfault!")"#);
}

Output:

$ cargo run
    Finished debug [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target\debug\lua-fail.exe`
error: Process didn't exit successfully: `target\debug\lua-fail.exe` (exit code: 3221225477)
Segmentation fault

This was found with #65.

therustmonk avatar Sep 02 '16 08:09 therustmonk

I experimented a bit with how to solve this issue conceptually. IMO, the cleanest solution is a separate Context type which is responsible for lua_openstate and lua_close, and from which a main State (which would correspond to Lua threads) can be obtained. State would need either a lifetime parameter or to be hidden behind a reference(*) which restricts it to exist no longer than the Context does.

Further, new_thread and to_thread would need to return some sort of guard which pins the Lua-side value (probably in the registry) so that it is not garbage collected by Lua; otherwise, even if the original Context still exists, the specific thread may have been destroyed.

I should note that it follows from this that State's methods need to be changed to work on &self rather than &mut self, otherwise functions like xmove become impossible to use.

(*) If T is zero-sized, it's legal to produce arbitrary &T and &mut T in unsafe code, since dereferencing them does nothing. Therefore we could produce a &State whose pointer is actually the lua_State. This is needed if we want Context to be able to Deref to State (so State methods can be called on Context directly), but not really important if that ability is not desired.

I have some of this stuff implemented in an experimental branch but it's a little messy; if I know which parts are important I can pick them out into PRs.

SpaceManiac avatar Sep 02 '16 23:09 SpaceManiac

It should also be noted that lua_newthread pushes the thread on the stack, but there seems to be no code that pops that value, potentially leading to unbounded stack growth. However, doing that would make the thread eligible for garbage collection.

jonas-schievink avatar Jul 21 '17 19:07 jonas-schievink