diff --git a/src/error.rs b/src/error.rs index 2787331e..2717530a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -30,7 +30,13 @@ pub enum Error { /// /// This is an error because `rlua` callbacks are FnMut and thus can only be mutably borrowed /// once. - RecursiveCallbackError, + RecursiveCallback, + /// Either a callback or a userdata method has been called, but the callback or userdata has + /// been destructed. + /// + /// This can happen either due to to being destructed in a previous __gc, or due to being + /// destructed from exiting a `Lua::scope` call. + CallbackDestructed, /// A Rust value could not be converted to a Lua value. ToLuaConversionError { /// Name of the Rust type that could not be converted. @@ -117,7 +123,11 @@ impl fmt::Display for Error { Error::GarbageCollectorError(ref msg) => { write!(fmt, "garbage collector error: {}", msg) } - Error::RecursiveCallbackError => write!(fmt, "callback called recursively"), + Error::RecursiveCallback => write!(fmt, "callback called recursively"), + Error::CallbackDestructed => write!( + fmt, + "a destructed callback or destructed userdata method was called" + ), Error::ToLuaConversionError { from, to, diff --git a/src/lua.rs b/src/lua.rs index de86ac80..1abc8b03 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -966,11 +966,6 @@ impl Lua { func: Callback<'callback>, ) -> Result> { unsafe extern "C" fn callback_call_impl(state: *mut ffi::lua_State) -> c_int { - if ffi::lua_type(state, ffi::lua_upvalueindex(1)) == ffi::LUA_TNIL { - ffi::lua_pushstring(state, cstr!("rust callback has been destructed")); - ffi::lua_error(state) - } - callback_error(state, || { let lua = Lua { state: state, @@ -978,10 +973,14 @@ impl Lua { ephemeral: true, }; + if ffi::lua_type(state, ffi::lua_upvalueindex(1)) == ffi::LUA_TNIL { + return Err(Error::CallbackDestructed); + } + let func = get_userdata::>(state, ffi::lua_upvalueindex(1)); let mut func = (*func) .try_borrow_mut() - .map_err(|_| Error::RecursiveCallbackError)?; + .map_err(|_| Error::RecursiveCallback)?; let nargs = ffi::lua_gettop(state); let mut args = MultiValue::new(); diff --git a/src/tests.rs b/src/tests.rs index cd661f41..63e5d262 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -5,7 +5,8 @@ use std::cell::Cell; use std::sync::Arc; use std::panic::catch_unwind; -use {Error, ExternalError, Function, Lua, Nil, Result, Table, UserData, Value, Variadic}; +use {Error, ExternalError, Function, Lua, Nil, Result, Table, UserData, UserDataMethods, Value, + Variadic}; #[test] fn test_load() { @@ -458,7 +459,7 @@ fn test_recursive_callback_error() { { Err(Error::CallbackError { ref cause, .. }) => match *cause.as_ref() { Error::CallbackError { ref cause, .. } => match *cause.as_ref() { - Error::RecursiveCallbackError { .. } => {} + Error::RecursiveCallback { .. } => {} ref other => panic!("incorrect result: {:?}", other), }, ref other => panic!("incorrect result: {:?}", other), @@ -619,19 +620,24 @@ fn scope_func() { assert_eq!(rc.get(), 42); assert_eq!(Rc::strong_count(&rc), 1); - assert!( - lua.globals() - .get::<_, Function>("bad") - .unwrap() - .call::<_, ()>(()) - .is_err() - ); + match lua.globals() + .get::<_, Function>("bad") + .unwrap() + .call::<_, ()>(()) + { + Err(Error::CallbackError { .. }) => {} + r => panic!("improper return for destructed function: {:?}", r), + }; } #[test] fn scope_drop() { struct MyUserdata(Rc<()>); - impl UserData for MyUserdata {} + impl UserData for MyUserdata { + fn add_methods(methods: &mut UserDataMethods) { + methods.add_method("method", |_, _, ()| Ok(())); + } + } let rc = Rc::new(()); @@ -646,6 +652,11 @@ fn scope_drop() { assert_eq!(Rc::strong_count(&rc), 2); }); assert_eq!(Rc::strong_count(&rc), 1); + + match lua.exec::<()>("test:method()", None) { + Err(Error::CallbackError { .. }) => {} + r => panic!("improper return for destructed userdata: {:?}", r), + }; } #[test] diff --git a/src/util.rs b/src/util.rs index d8345dba..a7035e65 100644 --- a/src/util.rs +++ b/src/util.rs @@ -24,29 +24,25 @@ pub unsafe fn stack_guard(state: *mut ffi::lua_State, change: c_int, op: F where F: FnOnce() -> R, { - if cfg!(debug_assertions) { - let expected = ffi::lua_gettop(state) + change; - lua_internal_assert!( - state, - expected >= 0, - "too many stack values would be popped" - ); + let expected = ffi::lua_gettop(state) + change; + lua_internal_assert!( + state, + expected >= 0, + "too many stack values would be popped" + ); - let res = op(); + let res = op(); - let top = ffi::lua_gettop(state); - lua_internal_assert!( - state, - ffi::lua_gettop(state) == expected, - "expected stack to be {}, got {}", - expected, - top - ); + let top = ffi::lua_gettop(state); + lua_internal_assert!( + state, + ffi::lua_gettop(state) == expected, + "expected stack to be {}, got {}", + expected, + top + ); - res - } else { - op() - } + res } // Run an operation on a lua_State and automatically clean up the stack before @@ -62,45 +58,36 @@ pub unsafe fn stack_err_guard(state: *mut ffi::lua_State, change: c_int, o where F: FnOnce() -> Result, { - if cfg!(debug_assertions) { - let expected = ffi::lua_gettop(state) + change; + let expected = ffi::lua_gettop(state) + change; + lua_internal_assert!( + state, + expected >= 0, + "too many stack values would be popped" + ); + + let res = op(); + + let top = ffi::lua_gettop(state); + if res.is_ok() { lua_internal_assert!( state, - expected >= 0, - "too many stack values would be popped" + ffi::lua_gettop(state) == expected, + "expected stack to be {}, got {}", + expected, + top ); - - let res = op(); - - let top = ffi::lua_gettop(state); - if res.is_ok() { - lua_internal_assert!( - state, - ffi::lua_gettop(state) == expected, - "expected stack to be {}, got {}", - expected, - top - ); - } else { - lua_internal_assert!( - state, - top >= expected, - "{} too many stack values popped", - top - expected - ); - if top > expected { - ffi::lua_settop(state, expected); - } - } - res } else { - let prev = ffi::lua_gettop(state) + change; - let res = op(); - if res.is_err() { - ffi::lua_settop(state, prev); + lua_internal_assert!( + state, + top >= expected, + "{} too many stack values popped", + top - expected + ); + if top > expected { + ffi::lua_settop(state, expected); } - res } + res } // Call a function that calls into the Lua API and may trigger a Lua error (longjmp) in a safe way. @@ -253,7 +240,6 @@ pub unsafe fn push_userdata(state: *mut ffi::lua_State, t: T) -> Result<()> { }) } -// Returns None in the case that the userdata has already been garbage collected. pub unsafe fn get_userdata(state: *mut ffi::lua_State, index: c_int) -> *mut T { let ud = ffi::lua_touserdata(state, index) as *mut T; lua_internal_assert!(state, !ud.is_null(), "userdata pointer is null"); @@ -269,9 +255,10 @@ pub unsafe fn take_userdata(state: *mut ffi::lua_State) -> T { // after the first call to __gc. get_destructed_userdata_metatable(state); ffi::lua_setmetatable(state, -2); - let ud = &mut *(ffi::lua_touserdata(state, -1) as *mut T); + let ud = ffi::lua_touserdata(state, -1) as *mut T; + lua_internal_assert!(state, !ud.is_null(), "userdata pointer is null"); ffi::lua_pop(state, 1); - mem::replace(ud, mem::uninitialized()) + ptr::read(ud) } pub unsafe extern "C" fn userdata_destructor(state: *mut ffi::lua_State) -> c_int { @@ -611,7 +598,7 @@ unsafe fn get_destructed_userdata_metatable(state: *mut ffi::lua_State) -> c_int static DESTRUCTED_USERDATA_METATABLE: u8 = 0; unsafe extern "C" fn destructed_error(state: *mut ffi::lua_State) -> c_int { - ffi::lua_pushstring(state, cstr!("userdata has been destructed")); + push_wrapped_error(state, Error::CallbackDestructed); ffi::lua_error(state) }