Make the `Scope` lifetimes more sensible

Avoids messy lifetime issues when interacting with other handle types with scope
produced values.

The whole lifetime situation with 'lua on most methods could actually probably
use some looking at, I'm sure it probably has lots of less than optimal
decisions in it.

This also adds a proper comment to the 'scope lifetime to explain that the key
is that 'scope needs to be invariant to make things safe.  Disregard my previous
commit message, the real problem is that I had a poor understanding of lifetime
variance / invaraince.
This commit is contained in:
kyren 2018-02-08 18:42:50 -05:00
parent 7701aeef85
commit b6bc8d0bed
2 changed files with 31 additions and 19 deletions

View File

@ -30,10 +30,11 @@ pub struct Lua {
/// Constructed by the `Lua::scope` method, allows temporarily passing to Lua userdata that is /// Constructed by the `Lua::scope` method, allows temporarily passing to Lua userdata that is
/// !Send, and callbacks that are !Send and not 'static. /// !Send, and callbacks that are !Send and not 'static.
pub struct Scope<'scope> { pub struct Scope<'lua, 'scope> {
lua: &'scope Lua, lua: &'lua Lua,
destructors: RefCell<Vec<Box<FnMut(*mut ffi::lua_State) -> Box<Any>>>>, destructors: RefCell<Vec<Box<FnMut(*mut ffi::lua_State) -> Box<Any>>>>,
_phantom: PhantomData<RefCell<&'scope ()>>, // 'scope lifetime must be invariant
_phantom: PhantomData<&'scope mut &'scope ()>,
} }
// Data associated with the main lua_State via lua_getextraspace. // Data associated with the main lua_State via lua_getextraspace.
@ -313,6 +314,7 @@ impl Lua {
/// Calls the given function with a `Scope` parameter, giving the function the ability to create /// Calls the given function with a `Scope` parameter, giving the function the ability to create
/// userdata from rust types that are !Send, and rust callbacks that are !Send and not 'static. /// userdata from rust types that are !Send, and rust callbacks that are !Send and not 'static.
///
/// The lifetime of any function or userdata created through `Scope` lasts only until the /// The lifetime of any function or userdata created through `Scope` lasts only until the
/// completion of this method call, on completion all such created values are automatically /// completion of this method call, on completion all such created values are automatically
/// dropped and Lua references to them are invalidated. If a script accesses a value created /// dropped and Lua references to them are invalidated. If a script accesses a value created
@ -320,16 +322,23 @@ impl Lua {
/// lifetime of values created through `Scope`, and we know that `Lua` cannot be sent to another /// lifetime of values created through `Scope`, and we know that `Lua` cannot be sent to another
/// thread while `Scope` is live, it is safe to allow !Send datatypes and functions whose /// thread while `Scope` is live, it is safe to allow !Send datatypes and functions whose
/// lifetimes only outlive the scope lifetime. /// lifetimes only outlive the scope lifetime.
pub fn scope<'scope, F, R>(&'scope self, f: F) -> R ///
/// To make the lifetimes work out, handles that `Lua::scope` produces have the `'lua` lifetime
/// of the parent `Lua` instance. This allows the handles to scoped values to escape the
/// callback, but this was possible anyway by going through Lua itself. This is safe to do, but
/// not useful, because after the scope is dropped, all references to scoped values, whether in
/// Lua or in rust, are invalidated. `Function` types will error when called, and `AnyUserData`
/// types will be typeless.
pub fn scope<'lua, 'scope, F, R>(&'lua self, f: F) -> R
where where
F: FnOnce(&mut Scope<'scope>) -> R, F: FnOnce(&Scope<'lua, 'scope>) -> R,
{ {
let mut scope = Scope { let scope = Scope {
lua: self, lua: self,
destructors: RefCell::new(Vec::new()), destructors: RefCell::new(Vec::new()),
_phantom: PhantomData, _phantom: PhantomData,
}; };
let r = f(&mut scope); let r = f(&scope);
drop(scope); drop(scope);
r r
} }
@ -1038,18 +1047,21 @@ impl Lua {
} }
} }
impl<'scope> Scope<'scope> { impl<'lua, 'scope> Scope<'lua, 'scope> {
pub fn create_function<A, R, F>(&self, mut func: F) -> Result<Function<'scope>> pub fn create_function<A, R, F>(&self, mut func: F) -> Result<Function<'lua>>
where where
A: FromLuaMulti<'scope>, A: FromLuaMulti<'lua>,
R: ToLuaMulti<'scope>, R: ToLuaMulti<'lua>,
F: 'scope + FnMut(&'scope Lua, A) -> Result<R>, F: 'scope + FnMut(&'lua Lua, A) -> Result<R>,
{ {
unsafe { unsafe {
let mut f = self.lua let f: Box<FnMut(&'lua Lua, MultiValue<'lua>) -> Result<MultiValue<'lua>>> = Box::new(
.create_callback_function(Box::new(move |lua, args| { move |lua, args| func(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua),
func(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua) );
}))?;
// SCARY, we are transmuting away the 'static requirement
let mut f = self.lua.create_callback_function(mem::transmute(f))?;
f.0.drop_unref = false; f.0.drop_unref = false;
let mut destructors = self.destructors.borrow_mut(); let mut destructors = self.destructors.borrow_mut();
let registry_id = f.0.registry_id; let registry_id = f.0.registry_id;
@ -1073,7 +1085,7 @@ impl<'scope> Scope<'scope> {
} }
} }
pub fn create_userdata<T>(&self, data: T) -> Result<AnyUserData<'scope>> pub fn create_userdata<T>(&self, data: T) -> Result<AnyUserData<'lua>>
where where
T: UserData, T: UserData,
{ {
@ -1096,7 +1108,7 @@ impl<'scope> Scope<'scope> {
} }
} }
impl<'scope> Drop for Scope<'scope> { impl<'lua, 'scope> Drop for Scope<'lua, 'scope> {
fn drop(&mut self) { fn drop(&mut self) {
// We separate the action of invalidating the userdata in Lua and actually dropping the // We separate the action of invalidating the userdata in Lua and actually dropping the
// userdata type into two phases. This is so that, in the event a userdata drop panics, we // userdata type into two phases. This is so that, in the event a userdata drop panics, we

View File

@ -40,7 +40,7 @@ impl Drop for RegistryKey {
} }
pub(crate) type Callback<'lua> = pub(crate) type Callback<'lua> =
Box<FnMut(&'lua Lua, MultiValue<'lua>) -> Result<MultiValue<'lua>> + 'lua>; Box<FnMut(&'lua Lua, MultiValue<'lua>) -> Result<MultiValue<'lua>>>;
pub(crate) struct LuaRef<'lua> { pub(crate) struct LuaRef<'lua> {
pub lua: &'lua Lua, pub lua: &'lua Lua,