michael/mlua
michael
/
mlua
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Simplify handling of userdata __gc and resurrected userdata. 

Now, simply remove the userdata table immediately before dropping the userdata.
This does two things, it prevents __gc from double dropping the userdata, and
after the first call to __gc, it prevents the userdata from being identified as
any particular userdata type, so it cannot be misused after being finalized.

This change thus removes the userdata invalidation error, and simplifies a lot
of userdata handling code.

It also fixes a panic bug.  Because there is no predictable order for
finalizers, it is possible to run a userdata finalizer that does not resurrect
itself before a lua table finalizer that accesses that userdata, and this means
that there were several asserts that were possible to trigger in normal Lua code
in util.rs related to `WrappedError`.

Now, finalized userdata is simply a userdata with no methods, so any use of
finalized userdata becomes a normal script runtime error (though, with a
potentially confusing error message).  As a future improvement, we could set
a metatable on finalized userdata that provides a better error message.
This commit is contained in:
kyren 2018-01-27 18:12:39 -05:00
parent cbc882bad0
commit 77eb73a50c
4 changed files with 50 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/error.rs
View File

@ -31,13 +31,6 @@ pub enum Error {
/// This is an error because `rlua` callbacks are FnMut and thus can only be mutably borrowed
/// once.
RecursiveCallbackError,
/// Lua code has accessed a [`UserData`] value that was already garbage collected.
///
/// This can happen when a [`UserData`] has a custom `__gc` metamethod, this method resurrects
/// the [`UserData`], and then the [`UserData`] is subsequently accessed.
///
/// [`UserData`]: trait.UserData.html
ExpiredUserData,
/// A Rust value could not be converted to a Lua value.
ToLuaConversionError {
/// Name of the Rust type that could not be converted.
@ -123,10 +116,6 @@ impl fmt::Display for Error {
write!(fmt, "garbage collector error: {}", msg)
}
Error::RecursiveCallbackError => write!(fmt, "callback called recursively"),
Error::ExpiredUserData => write!(
fmt,
"access of userdata which has already been garbage collected"
),
Error::ToLuaConversionError {
from,
to,

2
src/lua.rs
View File

@ -957,7 +957,7 @@ impl Lua {
ephemeral: true,
};
let func = get_userdata::<RefCell<Callback>>(state, ffi::lua_upvalueindex(1))?;
let func = get_userdata::<RefCell<Callback>>(state, ffi::lua_upvalueindex(1));
let mut func = (*func)
.try_borrow_mut()
.map_err(|_| Error::RecursiveCallbackError)?;

16
src/userdata.rs
View File

@ -386,7 +386,7 @@ impl<'lua> AnyUserData<'lua> {
ffi::lua_pop(lua.state, 3);
Err(Error::UserDataTypeMismatch)
} else {
let res = func(&*get_userdata::<RefCell<T>>(lua.state, -3)?);
let res = func(&*get_userdata::<RefCell<T>>(lua.state, -3));
ffi::lua_pop(lua.state, 3);
res
}
@ -434,7 +434,7 @@ impl<'lua> AnyUserData<'lua> {
#[cfg(test)]
mod tests {
use super::{MetaMethod, UserData, UserDataMethods};
use error::{Error, ExternalError};
use error::ExternalError;
use string::String;
use function::Function;
use lua::Lua;
@ -568,7 +568,7 @@ mod tests {
globals.set("userdata", MyUserdata { id: 123 }).unwrap();
}
match lua.eval::<()>(
assert!(lua.eval::<()>(
r#"
local tbl = setmetatable({
userdata = userdata
@ -582,14 +582,8 @@ mod tests {
collectgarbage("collect")
hatch:access()
"#,
None,
) {
Err(Error::CallbackError { cause, .. }) => match *cause {
Error::ExpiredUserData { .. } => {}
ref other => panic!("incorrect result: {}", other),
},
other => panic!("incorrect result: {:?}", other),
}
None
).is_err());
}
#[test]

73
src/util.rs
View File

@ -104,21 +104,25 @@ pub unsafe fn protect_lua_call<F, R>(
f: F,
) -> Result<R>
where
F: FnMut(*mut ffi::lua_State) -> R,
F: FnOnce(*mut ffi::lua_State) -> R,
{
struct Params<F, R> {
function: F,
ret: Option<R>,
result: R,
nresults: c_int,
}
unsafe extern "C" fn do_call<F, R>(state: *mut ffi::lua_State) -> c_int
where
F: FnMut(*mut ffi::lua_State) -> R,
F: FnOnce(*mut ffi::lua_State) -> R,
{
let params = ffi::lua_touserdata(state, -1) as *mut Params<F, R>;
ffi::lua_pop(state, 1);
(*params).ret = Some(((*params).function)(state));
let function = mem::replace(&mut (*params).function, mem::uninitialized());
mem::replace(&mut (*params).result, function(state));
// params now has function uninitialied and result initialized
if (*params).nresults == ffi::LUA_MULTRET {
ffi::lua_gettop(state)
} else {
@ -132,20 +136,32 @@ where
ffi::lua_pushcfunction(state, do_call::<F, R>);
ffi::lua_rotate(state, stack_start + 1, 2);
// We are about to do some really scary stuff with the Params structure, both because
// protect_lua_call is very hot, and becuase we would like to allow the function type to be
// FnOnce rather than FnMut. We are using Params here both to pass data to the callback and
// return data from the callback.
//
// params starts out with function initialized and result uninitialized, nresults is Copy so we
// don't care about it.
let mut params = Params {
function: f,
ret: None,
result: mem::uninitialized(),
nresults,
};
ffi::lua_pushlightuserdata(state, &mut params as *mut Params<F, R> as *mut c_void);
let ret = ffi::lua_pcall(state, nargs + 1, nresults, stack_start + 1);
let result = mem::replace(&mut params.result, mem::uninitialized());
// params now has both function and result uninitialized, so we need to forget it so Drop isn't
// run.
mem::forget(params);
ffi::lua_remove(state, stack_start + 1);
if ret == ffi::LUA_OK {
Ok(params.ret.unwrap())
Ok(result)
} else {
Err(pop_error(state, ret))
}
@ -164,8 +180,7 @@ pub unsafe fn pop_error(state: *mut ffi::lua_State, err_code: c_int) -> Error {
if let Some(err) = pop_wrapped_error(state) {
err
} else if is_wrapped_panic(state, -1) {
let panic = get_userdata::<WrappedPanic>(state, -1)
.expect("WrappedPanic was somehow resurrected after garbage collection");
let panic = get_userdata::<WrappedPanic>(state, -1);
if let Some(p) = (*panic).0.take() {
ffi::lua_settop(state, 0);
resume_unwind(p);
@ -220,26 +235,30 @@ pub unsafe fn push_string(state: *mut ffi::lua_State, s: &str) -> Result<()> {
// Internally uses 4 stack spaces, does not call checkstack
pub unsafe fn push_userdata<T>(state: *mut ffi::lua_State, t: T) -> Result<()> {
let mut t = Some(t);
protect_lua_call(state, 0, 1, |state| {
let ud = ffi::lua_newuserdata(state, mem::size_of::<Option<T>>()) as *mut Option<T>;
ptr::write(ud, t.take());
protect_lua_call(state, 0, 1, move |state| {
let ud = ffi::lua_newuserdata(state, mem::size_of::<T>()) as *mut T;
ptr::write(ud, t);
})
}
// Returns None in the case that the userdata has already been garbage collected.
pub unsafe fn get_userdata<T>(state: *mut ffi::lua_State, index: c_int) -> Result<*mut T> {
let ud = ffi::lua_touserdata(state, index) as *mut Option<T>;
pub unsafe fn get_userdata<T>(state: *mut ffi::lua_State, index: c_int) -> *mut T {
let ud = ffi::lua_touserdata(state, index) as *mut T;
lua_assert!(state, !ud.is_null(), "userdata pointer is null");
(*ud)
.as_mut()
.map(|v| v as *mut T)
.ok_or(Error::ExpiredUserData)
ud
}
pub unsafe extern "C" fn userdata_destructor<T>(state: *mut ffi::lua_State) -> c_int {
callback_error(state, || {
*(ffi::lua_touserdata(state, 1) as *mut Option<T>) = None;
// We clear the metatable of userdata on __gc so that it will not be double dropped, and
// also so that it cannot be used or identified as any particular userdata type after the
// first call to __gc.
gc_guard(state, || {
ffi::lua_newtable(state);
ffi::lua_setmetatable(state, -2);
});
let ud = &mut *(ffi::lua_touserdata(state, 1) as *mut T);
mem::replace(ud, mem::uninitialized());
Ok(0)
})
}
@ -367,9 +386,8 @@ pub unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: Error) {
ffi::luaL_checkstack(state, 2, ptr::null());
gc_guard(state, || {
let ud = ffi::lua_newuserdata(state, mem::size_of::<Option<WrappedError>>())
as *mut Option<WrappedError>;
ptr::write(ud, Some(WrappedError(err)))
let ud = ffi::lua_newuserdata(state, mem::size_of::<WrappedError>()) as *mut WrappedError;
ptr::write(ud, WrappedError(err))
});
get_error_metatable(state);
@ -382,8 +400,7 @@ pub unsafe fn pop_wrapped_error(state: *mut ffi::lua_State) -> Option<Error> {
if ffi::lua_gettop(state) == 0 || !is_wrapped_error(state, -1) {
None
} else {
let err = &*get_userdata::<WrappedError>(state, -1)
.expect("WrappedError was somehow resurrected after garbage collection");
let err = &*get_userdata::<WrappedError>(state, -1);
let err = err.0.clone();
ffi::lua_pop(state, 1);
Some(err)
@ -415,9 +432,8 @@ unsafe fn push_wrapped_panic(state: *mut ffi::lua_State, panic: Box<Any + Send>)
ffi::luaL_checkstack(state, 2, ptr::null());
gc_guard(state, || {
let ud = ffi::lua_newuserdata(state, mem::size_of::<Option<WrappedPanic>>())
as *mut Option<WrappedPanic>;
ptr::write(ud, Some(WrappedPanic(Some(panic))))
let ud = ffi::lua_newuserdata(state, mem::size_of::<WrappedPanic>()) as *mut WrappedPanic;
ptr::write(ud, WrappedPanic(Some(panic)))
});
get_panic_metatable(state);
@ -480,8 +496,7 @@ unsafe fn get_error_metatable(state: *mut ffi::lua_State) -> c_int {
unsafe extern "C" fn error_tostring(state: *mut ffi::lua_State) -> c_int {
callback_error(state, || {
if is_wrapped_error(state, -1) {
let error = get_userdata::<WrappedError>(state, -1)
.expect("WrappedError was somehow resurrected after garbage collection");
let error = get_userdata::<WrappedError>(state, -1);
let error_str = (*error).0.to_string();
gc_guard(state, || {
ffi::lua_pushlstring(