more refactoring trying to find a workable path to 'm' error safety

This commit is contained in:
kyren 2017-12-03 18:15:31 -05:00
parent 5742e3f20a
commit e41c72d642
2 changed files with 92 additions and 83 deletions

View File

@ -205,21 +205,23 @@ impl<'lua> Function<'lua> {
let nargs = args.len() as c_int; let nargs = args.len() as c_int;
check_stack(lua.state, nargs + 3); check_stack(lua.state, nargs + 3);
ffi::lua_pushcfunction(lua.state, error_traceback);
let stack_start = ffi::lua_gettop(lua.state); let stack_start = ffi::lua_gettop(lua.state);
lua.push_ref(lua.state, &self.0); lua.push_ref(lua.state, &self.0);
for arg in args { for arg in args {
lua.push_value(lua.state, arg); lua.push_value(lua.state, arg);
} }
handle_error( let ret = ffi::lua_pcall(lua.state, nargs, ffi::LUA_MULTRET, stack_start);
lua.state, if ret != ffi::LUA_OK {
pcall_with_traceback(lua.state, nargs, ffi::LUA_MULTRET), return Err(pop_error(lua.state, ret));
)?; }
let nresults = ffi::lua_gettop(lua.state) - stack_start; let nresults = ffi::lua_gettop(lua.state) - stack_start;
let mut results = MultiValue::new(); let mut results = MultiValue::new();
check_stack(lua.state, 1); check_stack(lua.state, 1);
for _ in 0..nresults { for _ in 0..nresults {
results.push_front(lua.pop_value(lua.state)); results.push_front(lua.pop_value(lua.state));
} }
ffi::lua_pop(lua.state, 1);
R::from_lua_multi(results, lua) R::from_lua_multi(results, lua)
}) })
} }
@ -391,10 +393,11 @@ impl<'lua> Thread<'lua> {
lua.push_value(thread_state, arg); lua.push_value(thread_state, arg);
} }
handle_error( let ret = ffi::lua_resume(thread_state, lua.state, nargs);
thread_state, if ret != ffi::LUA_OK && ret != ffi::LUA_YIELD {
resume_with_traceback(thread_state, lua.state, nargs), error_traceback(thread_state);
)?; return Err(pop_error(thread_state, ret));
}
let nresults = ffi::lua_gettop(thread_state); let nresults = ffi::lua_gettop(thread_state);
let mut results = MultiValue::new(); let mut results = MultiValue::new();
@ -442,6 +445,14 @@ impl Drop for Lua {
fn drop(&mut self) { fn drop(&mut self) {
unsafe { unsafe {
if !self.ephemeral { if !self.ephemeral {
if cfg!(test) {
let top = ffi::lua_gettop(self.state);
if top != 0 {
eprintln!("Lua stack leak detected, stack top is {}", top);
::std::process::abort()
}
}
ffi::lua_close(self.state); ffi::lua_close(self.state);
} }
} }
@ -578,9 +589,7 @@ impl Lua {
stack_err_guard(self.state, 0, || { stack_err_guard(self.state, 0, || {
check_stack(self.state, 1); check_stack(self.state, 1);
handle_error( match if let Some(name) = name {
self.state,
if let Some(name) = name {
let name = CString::new(name.to_owned()).map_err(|e| { let name = CString::new(name.to_owned()).map_err(|e| {
Error::ToLuaConversionError { Error::ToLuaConversionError {
from: "&str", from: "&str",
@ -601,10 +610,10 @@ impl Lua {
source.len(), source.len(),
ptr::null(), ptr::null(),
) )
}, } {
)?; ffi::LUA_OK => Ok(Function(self.pop_ref(self.state))),
err => Err(pop_error(self.state, err)),
Ok(Function(self.pop_ref(self.state))) }
}) })
} }
} }

View File

@ -107,8 +107,10 @@ pub unsafe fn pgettable(state: *mut ffi::lua_State, index: c_int) -> Result<c_in
ffi::lua_pushvalue(state, -3); ffi::lua_pushvalue(state, -3);
ffi::lua_remove(state, -4); ffi::lua_remove(state, -4);
handle_error(state, pcall_with_traceback(state, 2, 1))?; match pcall_with_traceback(state, 2, 1) {
Ok(ffi::lua_type(state, -1)) ffi::LUA_OK => Ok(ffi::lua_type(state, -1)),
err => Err(pop_error(state, err)),
}
} }
// Protected version of lua_settable, uses 4 stack spaces, does not call checkstack. // Protected version of lua_settable, uses 4 stack spaces, does not call checkstack.
@ -127,8 +129,10 @@ pub unsafe fn psettable(state: *mut ffi::lua_State, index: c_int) -> Result<()>
ffi::lua_remove(state, -5); ffi::lua_remove(state, -5);
ffi::lua_remove(state, -5); ffi::lua_remove(state, -5);
handle_error(state, pcall_with_traceback(state, 3, 0))?; match pcall_with_traceback(state, 3, 0) {
Ok(()) ffi::LUA_OK => Ok(()),
err => Err(pop_error(state, err)),
}
} }
// Protected version of luaL_len, uses 2 stack spaces, does not call checkstack. // Protected version of luaL_len, uses 2 stack spaces, does not call checkstack.
@ -143,11 +147,15 @@ pub unsafe fn plen(state: *mut ffi::lua_State, index: c_int) -> Result<ffi::lua_
ffi::lua_pushcfunction(state, len); ffi::lua_pushcfunction(state, len);
ffi::lua_pushvalue(state, table_index); ffi::lua_pushvalue(state, table_index);
handle_error(state, pcall_with_traceback(state, 1, 1))?; match pcall_with_traceback(state, 1, 1) {
ffi::LUA_OK => {
let len = ffi::lua_tointeger(state, -1); let len = ffi::lua_tointeger(state, -1);
ffi::lua_pop(state, 1); ffi::lua_pop(state, 1);
Ok(len) Ok(len)
} }
err => Err(pop_error(state, err)),
}
}
// Protected version of lua_geti, uses 3 stack spaces, does not call checkstack. // Protected version of lua_geti, uses 3 stack spaces, does not call checkstack.
pub unsafe fn pgeti( pub unsafe fn pgeti(
@ -167,8 +175,10 @@ pub unsafe fn pgeti(
ffi::lua_pushvalue(state, table_index); ffi::lua_pushvalue(state, table_index);
ffi::lua_pushinteger(state, i); ffi::lua_pushinteger(state, i);
handle_error(state, pcall_with_traceback(state, 2, 1))?; match pcall_with_traceback(state, 2, 1) {
Ok(ffi::lua_type(state, -1)) ffi::LUA_OK => Ok(ffi::lua_type(state, -1)),
err => Err(pop_error(state, err)),
}
} }
// Protected version of lua_next, uses 3 stack spaces, does not call checkstack. // Protected version of lua_next, uses 3 stack spaces, does not call checkstack.
@ -189,7 +199,8 @@ pub unsafe fn pnext(state: *mut ffi::lua_State, index: c_int) -> Result<c_int> {
ffi::lua_remove(state, -4); ffi::lua_remove(state, -4);
let stack_start = ffi::lua_gettop(state) - 3; let stack_start = ffi::lua_gettop(state) - 3;
handle_error(state, pcall_with_traceback(state, 2, ffi::LUA_MULTRET))?; match pcall_with_traceback(state, 2, ffi::LUA_MULTRET) {
ffi::LUA_OK => {
let nresults = ffi::lua_gettop(state) - stack_start; let nresults = ffi::lua_gettop(state) - stack_start;
if nresults == 0 { if nresults == 0 {
Ok(0) Ok(0)
@ -197,17 +208,22 @@ pub unsafe fn pnext(state: *mut ffi::lua_State, index: c_int) -> Result<c_int> {
Ok(1) Ok(1)
} }
} }
err => Err(pop_error(state, err)),
}
}
// If the return code indicates an error, pops the error off of the stack and // Pops an error off of the stack and returns it. If the error is actually a WrappedPanic, clears
// returns Err. If the error is actually a WrappedPanic, clears the current lua // the current lua stack and continues the panic. If the error on the top of the stack is actually
// stack and continues the panic. If the error on the top of the stack is // a WrappedError, just returns it. Otherwise, interprets the error as the appropriate lua error.
// actually a WrappedError, just returns it. Otherwise, interprets the error as pub unsafe fn pop_error(state: *mut ffi::lua_State, err_code: c_int) -> Error {
// the appropriate lua error. lua_assert!(
pub unsafe fn handle_error(state: *mut ffi::lua_State, err: c_int) -> Result<()> { state,
if err == ffi::LUA_OK || err == ffi::LUA_YIELD { err_code != ffi::LUA_OK && err_code != ffi::LUA_YIELD,
Ok(()) "pop_error called with non-error return code"
} else if let Some(err) = pop_wrapped_error(state) { );
Err(err)
if let Some(err) = pop_wrapped_error(state) {
err
} else if is_wrapped_panic(state, -1) { } else if is_wrapped_panic(state, -1) {
let panic = get_userdata::<WrappedPanic>(state, -1); let panic = get_userdata::<WrappedPanic>(state, -1);
if let Some(p) = (*panic).0.take() { if let Some(p) = (*panic).0.take() {
@ -227,7 +243,7 @@ pub unsafe fn handle_error(state: *mut ffi::lua_State, err: c_int) -> Result<()>
}; };
ffi::lua_pop(state, 1); ffi::lua_pop(state, 1);
Err(match err { match err_code {
ffi::LUA_ERRRUN => Error::RuntimeError(err_string), ffi::LUA_ERRRUN => Error::RuntimeError(err_string),
ffi::LUA_ERRSYNTAX => { ffi::LUA_ERRSYNTAX => {
Error::SyntaxError { Error::SyntaxError {
@ -258,7 +274,7 @@ pub unsafe fn handle_error(state: *mut ffi::lua_State, err: c_int) -> Result<()>
process::abort() process::abort()
} }
_ => lua_panic!(state, "internal error: unrecognized lua error code"), _ => lua_panic!(state, "internal error: unrecognized lua error code"),
}) }
} }
} }
@ -293,7 +309,7 @@ pub unsafe extern "C" fn userdata_destructor<T>(state: *mut ffi::lua_State) -> c
// In the context of a lua callback, this will call the given function and if the given function // In the context of a lua callback, this will call the given function and if the given function
// returns an error, *or if the given function panics*, this will result in a call to lua_error (a // returns an error, *or if the given function panics*, this will result in a call to lua_error (a
// longjmp). The error or panic is wrapped in such a way that when calling handle_error back on // longjmp). The error or panic is wrapped in such a way that when calling pop_error back on
// the rust side, it will resume the panic. // the rust side, it will resume the panic.
pub unsafe fn callback_error<R, F>(state: *mut ffi::lua_State, f: F) -> R pub unsafe fn callback_error<R, F>(state: *mut ffi::lua_State, f: F) -> R
where where
@ -313,8 +329,8 @@ where
} }
// Takes an error at the top of the stack, and if it is a WrappedError, converts it to an // Takes an error at the top of the stack, and if it is a WrappedError, converts it to an
// Error::CallbackError with a traceback. If it is a lua error, creates a new string error with a // Error::CallbackError with a traceback, if it is some lua type, prints the error along with a
// printed traceback, and if it is a WrappedPanic, does not modify it. // traceback, and if it is a WrappedPanic, does not modify it.
pub unsafe extern "C" fn error_traceback(state: *mut ffi::lua_State) -> c_int { pub unsafe extern "C" fn error_traceback(state: *mut ffi::lua_State) -> c_int {
if let Some(error) = pop_wrapped_error(state) { if let Some(error) = pop_wrapped_error(state) {
ffi::luaL_traceback(state, state, ptr::null(), 0); ffi::luaL_traceback(state, state, ptr::null(), 0);
@ -345,11 +361,7 @@ pub unsafe extern "C" fn error_traceback(state: *mut ffi::lua_State) -> c_int {
// ffi::lua_pcall with a message handler that gives a nice traceback. If the // ffi::lua_pcall with a message handler that gives a nice traceback. If the
// caught error is actually a Error, will simply pass the error along. Does // caught error is actually a Error, will simply pass the error along. Does
// not call checkstack, and uses 2 extra stack spaces. // not call checkstack, and uses 2 extra stack spaces.
pub unsafe fn pcall_with_traceback( unsafe fn pcall_with_traceback(state: *mut ffi::lua_State, nargs: c_int, nresults: c_int) -> c_int {
state: *mut ffi::lua_State,
nargs: c_int,
nresults: c_int,
) -> c_int {
let msgh_position = ffi::lua_gettop(state) - nargs; let msgh_position = ffi::lua_gettop(state) - nargs;
ffi::lua_pushcfunction(state, error_traceback); ffi::lua_pushcfunction(state, error_traceback);
ffi::lua_insert(state, msgh_position); ffi::lua_insert(state, msgh_position);
@ -358,18 +370,6 @@ pub unsafe fn pcall_with_traceback(
ret ret
} }
pub unsafe fn resume_with_traceback(
state: *mut ffi::lua_State,
from: *mut ffi::lua_State,
nargs: c_int,
) -> c_int {
let res = ffi::lua_resume(state, from, nargs);
if res != ffi::LUA_OK && res != ffi::LUA_YIELD {
error_traceback(state);
}
res
}
// A variant of pcall that does not allow lua to catch panic errors from callback_error // A variant of pcall that does not allow lua to catch panic errors from callback_error
pub unsafe extern "C" fn safe_pcall(state: *mut ffi::lua_State) -> c_int { pub unsafe extern "C" fn safe_pcall(state: *mut ffi::lua_State) -> c_int {
let top = ffi::lua_gettop(state); let top = ffi::lua_gettop(state);