I'm pretty sure you can cause UB in safe code with Callbacks

[kyren]

Callbacks are not currently placed inside a RefCell, and are also FnMuts (not Fns), so if you call a callback inside itself, you've got mutability and aliasing.

I think all userdatas should be inside RefCell to ensure this problem is avoided in the future, we could also provide the ability to add Fn (not FnMut) callbacks, but it kind of doubles the API surface area, unless we do some trait trickery. I think just using RefCell is fine for the moment.

[jonas-schievink]

One thing to keep in mind is that callbacks must currently outlive the 'static lifetime, so they can't really mutably borrow anything that would cause interesting UB (for example, mutable access to statics is already unsafe). Of course, this doesn't mean that there's no issue here, it's just not a particularly dangerous one.

[kyren]

I think they can move capture a value mutably, then get a reference to themselves via Lua and cause some pretty bad juju right? I can at least see how to mutate a value I currently hold a &mut reference to.

[jonas-schievink]

Ugh, of course. How could I miss that? Clearly I need to write more Rust.

Demonstration of UB:

extern crate rlua;

use rlua::*;

fn main() {
    let lua = Lua::new();

    let mut v = Some(Box::new(123));
    let f = lua.create_function::<_, (), _>(move |lua, mutate: bool| {
        if mutate {
            v = None;
        } else {
            let r = v.as_mut().unwrap();
            println!("BEFORE: first value at {:p}", r as *mut _);
            println!("BEFORE: first value is {}", r);
            lua.globals().get::<_, Function>("f").unwrap().call::<_, ()>(true).unwrap();
            println!("AFTER: first value at {:p}", r as *mut _);
            println!("AFTER: first value is {}", r);
        }

        Ok(())
    });
    lua.globals().set("f", f).unwrap();
    lua.globals().get::<_, Function>("f").unwrap().call::<_, ()>(false).unwrap();
}

Prints on my machine:

BEFORE: first value at 0x7f6feb021010
BEFORE: first value is 123
AFTER: first value at 0x7f6feb021010

...and then segfaults

[kyren]

Nice, thank you for the demonstration! Luckily, it should be a pretty easy fix.

[kyren]

I think this is fixed now with 4b7a340. I know that this causes a panic, and not an error, and that that is not ideal. It's actually not hard to make it an error, but it really should be another Error enum entry, and the panic.. kind of matches what happens with resurrected userdata? I don't know, I could have made it an Error, and I actually will... I plan on working on fixing #38 really soon, and doing all the API breakage at once.