Improve panic handling (check for twice resumed panics)

3 files changed, 45 insertions, 32 deletions

diff --git a/src/error.rs b/src/error.rs
index ad58307..029d629 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -139,6 +139,11 @@ pub enum Error {
         /// Original error returned by the Rust code.
         cause: Arc<Error>,
     },
+    /// A Rust panic that was previosly resumed, returned again.
+    ///
+    /// This error can occur only when a Rust panic resumed previously was recovered
+    /// and returned again.
+    PreviouslyResumedPanic,
     /// Serialization error.
     #[cfg(feature = "serialize")]
     #[cfg_attr(docsrs, doc(cfg(feature = "serialize")))]
@@ -230,6 +235,9 @@ impl fmt::Display for Error {
             Error::CallbackError { ref traceback, .. } => {
                 write!(fmt, "callback error: {}", traceback)
             }
+            Error::PreviouslyResumedPanic => {
+                write!(fmt, "previously resumed panic returned again")
+            }
             #[cfg(feature = "serialize")]
             Error::SerializeError(ref err) => {
                 write!(fmt, "serialize error: {}", err)
diff --git a/src/lua.rs b/src/lua.rs
index 9e94e19..e215201 100644
--- a/src/lua.rs
+++ b/src/lua.rs
@@ -4,6 +4,7 @@ use std::collections::HashMap;
 use std::ffi::CString;
 use std::marker::PhantomData;
 use std::os::raw::{c_char, c_int, c_void};
+use std::panic::resume_unwind;
 use std::sync::{Arc, Mutex, Weak};
 use std::{mem, ptr, str};
 
@@ -25,7 +26,7 @@ use crate::util::{
     assert_stack, callback_error, check_stack, get_gc_userdata, get_main_state, get_userdata,
     get_wrapped_error, init_error_registry, init_gc_metatable_for, init_userdata_metatable,
     pop_error, protect_lua, protect_lua_closure, push_gc_userdata, push_meta_gc_userdata,
-    push_string, push_userdata, push_wrapped_error, StackGuard,
+    push_string, push_userdata, push_wrapped_error, StackGuard, WrappedPanic,
 };
 use crate::value::{FromLua, FromLuaMulti, MultiValue, Nil, ToLua, ToLuaMulti, Value};
 
@@ -1402,32 +1403,33 @@ impl Lua {
 
     // Uses 2 stack spaces, does not call checkstack
     pub(crate) unsafe fn pop_value(&self) -> Value {
-        match ffi::lua_type(self.state, -1) {
+        let state = self.state;
+        match ffi::lua_type(state, -1) {
             ffi::LUA_TNIL => {
-                ffi::lua_pop(self.state, 1);
+                ffi::lua_pop(state, 1);
                 Nil
             }
 
             ffi::LUA_TBOOLEAN => {
-                let b = Value::Boolean(ffi::lua_toboolean(self.state, -1) != 0);
-                ffi::lua_pop(self.state, 1);
+                let b = Value::Boolean(ffi::lua_toboolean(state, -1) != 0);
+                ffi::lua_pop(state, 1);
                 b
             }
 
             ffi::LUA_TLIGHTUSERDATA => {
-                let ud = Value::LightUserData(LightUserData(ffi::lua_touserdata(self.state, -1)));
-                ffi::lua_pop(self.state, 1);
+                let ud = Value::LightUserData(LightUserData(ffi::lua_touserdata(state, -1)));
+                ffi::lua_pop(state, 1);
                 ud
             }
 
             ffi::LUA_TNUMBER => {
-                if ffi::lua_isinteger(self.state, -1) != 0 {
-                    let i = Value::Integer(ffi::lua_tointeger(self.state, -1));
-                    ffi::lua_pop(self.state, 1);
+                if ffi::lua_isinteger(state, -1) != 0 {
+                    let i = Value::Integer(ffi::lua_tointeger(state, -1));
+                    ffi::lua_pop(state, 1);
                     i
                 } else {
-                    let n = Value::Number(ffi::lua_tonumber(self.state, -1));
-                    ffi::lua_pop(self.state, 1);
+                    let n = Value::Number(ffi::lua_tonumber(state, -1));
+                    ffi::lua_pop(state, 1);
                     n
                 }
             }
@@ -1439,12 +1441,20 @@ impl Lua {
             ffi::LUA_TFUNCTION => Value::Function(Function(self.pop_ref())),
 
             ffi::LUA_TUSERDATA => {
-                // It should not be possible to interact with userdata types other than custom
-                // UserData types OR a WrappedError.  WrappedPanic should not be here.
-                if let Some(err) = get_wrapped_error(self.state, -1).as_ref() {
+                // We must prevent interaction with userdata types other than UserData OR a WrappedError.
+                // WrappedPanics are automatically resumed.
+                if let Some(err) = get_wrapped_error(state, -1).as_ref() {
                     let err = err.clone();
-                    ffi::lua_pop(self.state, 1);
+                    ffi::lua_pop(state, 1);
                     Value::Error(err)
+                } else if let Some(panic) = get_gc_userdata::<WrappedPanic>(state, -1).as_mut() {
+                    if let Some(panic) = (*panic).0.take() {
+                        ffi::lua_pop(state, 1);
+                        resume_unwind(panic);
+                    }
+                    // Previously resumed panic?
+                    ffi::lua_pop(state, 1);
+                    Nil
                 } else {
                     Value::UserData(AnyUserData(self.pop_ref()))
                 }
diff --git a/src/util.rs b/src/util.rs
index ae668fc..b2c2b61 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -186,7 +186,7 @@ pub unsafe fn pop_error(state: *mut ffi::lua_State, err_code: c_int) -> Error {
         if let Some(p) = (*panic).0.take() {
             resume_unwind(p);
         } else {
-            mlua_panic!("error during panic handling, panic was resumed twice")
+            Error::PreviouslyResumedPanic
         }
     } else {
         let err_string = to_string(state, -1).into_owned();
@@ -587,27 +587,22 @@ pub unsafe fn init_error_registry(state: *mut ffi::lua_State) {
                 Ok(err_buf)
             } else if let Some(panic) = get_gc_userdata::<WrappedPanic>(state, -1).as_ref() {
                 if let Some(ref p) = (*panic).0 {
-                    ffi::lua_pushlightuserdata(
-                        state,
-                        &ERROR_PRINT_BUFFER_KEY as *const u8 as *mut c_void,
-                    );
-                    ffi::lua_rawget(state, ffi::LUA_REGISTRYINDEX);
+                    let err_buf_key = &ERROR_PRINT_BUFFER_KEY as *const u8 as *const c_void;
+                    ffi::lua_rawgetp(state, ffi::LUA_REGISTRYINDEX, err_buf_key);
                     let err_buf = ffi::lua_touserdata(state, -1) as *mut String;
+                    (*err_buf).clear();
                     ffi::lua_pop(state, 2);
 
-                    let error = if let Some(x) = p.downcast_ref::<&str>() {
-                        x.to_string()
-                    } else if let Some(x) = p.downcast_ref::<String>() {
-                        x.to_string()
+                    if let Some(msg) = p.downcast_ref::<&str>() {
+                        let _ = write!(&mut (*err_buf), "{}", msg);
+                    } else if let Some(msg) = p.downcast_ref::<String>() {
+                        let _ = write!(&mut (*err_buf), "{}", msg);
                     } else {
-                        "panic".to_string()
+                        let _ = write!(&mut (*err_buf), "<panic>");
                     };
-
-                    (*err_buf).clear();
-                    let _ = write!(&mut (*err_buf), "{}", error);
                     Ok(err_buf)
                 } else {
-                    mlua_panic!("error during panic handling, panic was resumed")
+                    Err(Error::PreviouslyResumedPanic)
                 }
             } else {
                 // I'm not sure whether this is possible to trigger without bugs in mlua?
@@ -721,7 +716,7 @@ pub unsafe fn init_error_registry(state: *mut ffi::lua_State) {
 }
 
 struct WrappedError(pub Error);
-struct WrappedPanic(pub Option<Box<dyn Any + Send + 'static>>);
+pub(crate) struct WrappedPanic(pub Option<Box<dyn Any + Send + 'static>>);
 
 // Converts the given lua value to a string in a reasonable format without causing a Lua error or
 // panicking.