Couple of changes:

- Update readme, changelog, cargo version number in preparation for release
- Remove panicking behavior on recursive callback calls, add additional error
  variant for recursive callback errors.

CHANGELOG.md

@@ -1,3 +1,9 @@
+## [0.10.0]
+- Handle all 'm' functions in the Lua C API correctly, remove LUA_ERRGCMM hacks.
+- Lots and lots of internal changes to support handling all 'm' errors
+- Change the API in a lot of places due to functions that can trigger the gc now
+  potentially causing Error::GarbageCollectorError errors.
+
 ## [0.9.7]
 - Add unsafe function to load the debug Lua module (thanks @Timidger!)
 - Fix setmetatable wrapper with nil metatable (thanks again to @Timidger!)

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rlua"
-version = "0.9.7"
+version = "0.10.0"
 authors = ["kyren <catherine@chucklefish.org>"]
 description = "High level bindings to Lua 5.3"
 repository = "https://github.com/chucklefish/rlua"

README.md

@@ -36,9 +36,8 @@ allows for a much more flexible API.
 
 There are currently a few notable missing pieces of this API:
 
-  * Complete panic / abort safety for scripts.  This is a near term goal, but
-    currently there are ways to cause panics / aborts with lua scripts which are
-    not considered a bug.
+  * Complete panic / abort safety.  This is a near term goal, but currently
+    there are ways to cause panics / aborts with the API and with lua scripts.
   * Security limits on Lua code such as total instruction limits and control
     over which potentially dangerous libraries (e.g. io) are available to
     scripts.
@@ -78,12 +77,11 @@ safety level of the crate "Work In Progress".  Still, UB is considered the most
 serious kind of bug, so if you find the ability to cause UB with this API *at
 all*, please file a bug report.
 
-There are, however, a few ways to cause *panics* and even *aborts* with this API
-that are not currently considered bugs.  Usually these panics or aborts are
-alternatives to what would otherwise be unsafety.  A near term goal of this
-project is to remove the ability for lua to cause a panic or abort, and then
-panic / abort behavior will be considered a bug just like UB is, but that is
-currently not the case.
+There are, however, currently a few known ways to cause *panics* and even
+*aborts* with this API.  There is a near term goal to completely eliminate all
+ways to cause panics / aborts from scripts, so many of these can be considered
+bugs, but since they're known only file a bug repor if you notice any behavior
+that does not match what's described here.
 
 Panic / abort considerations when using this API:
 
@@ -102,33 +100,29 @@ Panic / abort considerations when using this API:
     `LUA_USE_APICHECK` it would generally be unsafe.
   * The library internally calls lua_checkstack to ensure that there is
     sufficient stack space, and if the stack cannot be sufficiently grown this
-    is a panic.  There should not be a way to cause this using the API, if you
-    encounter this, it is a bug.
-  * This API attempts only to handle errors in Lua C API functions that can
-    cause an error either directly or by running arbitrary Lua code directly,
-    not functions that can cause memory errors (marked as 'm' in the Lua C API
-    docs).  This means that we must take care to ensure that gc or memory errors
-    cannot occur, because this would unsafely longjmp potentially across rust
-    frames.  The allocator provided to lua is libc::malloc with an extra guard
-    to ensure that OOM errors are immediate aborts, because otherwise this would
-    be unsafe.  Similarly, 'setmetatable' is wrapped so that any `__gc`
-    metamethod specified in lua scripts will *abort* if the metamethod causes an
-    error rather than longjmp like a normal error would.  Lua objects can also
-    be resurrected with user provided `__gc` metamethods
-    (See [here](https://www.lua.org/manual/5.3/manual.html#2.5.1) for details),
-    and this includes userdata, so it is possible to trigger a panic from lua by
-    resurrecting a userdata and re-using it after it has been garbage collected.
-    It is an eventual goal of the library to ensure that lua scripts cannot
-    cause panics or aborts, but currently this is not true and this is a known
-    limitation.  Lua scripts should NOT be able to cause unsafety, though, this
-    is always considered a bug.
+    is a panic.  There should not be a way to cause this using this API, and if
+    you encounter this, it is a bug.
+  * Previous to version 0.10, `rlua` had a complicated system to guard against
+    LUA_ERRGCMM, and this system could cause aborts.  This is no longer the case
+    as of version 0.10, `rlua` now attempts to handle all errors that the Lua C
+    API can generate, including functions that can cause memory errors (any
+    function marked as 'v', 'e', or 'm' in the Lua C API docs).  This is,
+    however, extremely complicated and difficult to test, so if there is any
+    indication when using this API that a Lua error (longjmp) is being triggered
+    without being turned into an `rlua::Error`, *please please report this as a
+    bug*.
+  * The internal Lua allocator is set to use `realloc` from `libc`, but it is
+    wrapped in such a way that OOM errors are guaranteed to abort.  This is not
+    such a big deal, as this matches the behavior of rust itself.  This allows
+    the internals of `rlua` to, in certain rare cases, call 'm' Lua C API
+    functions with the garbage collector disabled and know that these cannot
+    error.  Though this is not a problem now, this will need to be changed in
+    order to add allocation limits to Lua instances.
   * There are currently no recursion limits on callbacks.  This could cause one
     of two problems, either the API will run out of stack space and cause a
     panic in Rust, or more likely it will cause an internal `LUA_USE_APICHECK`
     abort, from exceeding LUAI_MAXCCALLS.  This may be a source of unsafety if
     `LUA_USE_APICHECK` is disabled, and is considered a bug.
-  * All callbacks in `rlua` are FnMut, so if you trigger your own callback
-    recursively, currently this will panic.
   * There are currently no checks on argument sizes, and I think you may be able
     to cause an abort by providing a large enough `rlua::Variadic`.  I believe
     this would be unsafe without `LUA_USE_APICHECK` and should be considered a

src/error.rs

@@ -26,6 +26,11 @@ pub enum Error {
     ///
     /// The Lua VM returns this error when there is an error running a `__gc` metamethod.
     GarbageCollectorError(String),
+    /// A callback has triggered Lua code that has called the same callback again.
+    ///
+    /// This is an error, since `rlua` callbacks are FnMut an the functions can only be mutably
+    /// borrowed once.
+    RecursiveCallbackError,
     /// A Rust value could not be converted to a Lua value.
     ToLuaConversionError {
         /// Name of the Rust type that could not be converted.
@@ -110,6 +115,7 @@ impl fmt::Display for Error {
             Error::GarbageCollectorError(ref msg) => {
                 write!(fmt, "garbage collector error: {}", msg)
             }
+            Error::RecursiveCallbackError => write!(fmt, "callback called recursively"),
             Error::ToLuaConversionError {
                 from,
                 to,
@@ -150,6 +156,7 @@ impl StdError for Error {
             Error::SyntaxError { .. } => "syntax error",
             Error::RuntimeError(_) => "runtime error",
             Error::GarbageCollectorError(_) => "garbage collector error",
+            Error::RecursiveCallbackError => "callback called recursively",
             Error::ToLuaConversionError { .. } => "conversion error to lua",
             Error::FromLuaConversionError { .. } => "conversion error from lua",
             Error::CoroutineInactive => "attempt to resume inactive coroutine",

src/lua.rs

@@ -799,14 +799,9 @@ impl Lua {
                 };
 
                 let func = get_userdata::<RefCell<Callback>>(state, ffi::lua_upvalueindex(1));
-                let mut func = if let Ok(func) = (*func).try_borrow_mut() {
-                    func
-                } else {
-                    lua_panic!(
-                        state,
-                        "recursive callback function call would mutably borrow function twice"
-                    );
-                };
+                let mut func = (*func)
+                    .try_borrow_mut()
+                    .map_err(|_| Error::RecursiveCallbackError)?;
 
                 let nargs = ffi::lua_gettop(state);
                 let mut args = MultiValue::new();

src/tests.rs

@@ -428,8 +428,7 @@ fn test_pcall_xpcall() {
 }
 
 #[test]
-#[should_panic]
-fn test_recursive_callback_panic() {
+fn test_recursive_callback_error() {
     let lua = Lua::new();
 
     let mut v = Some(Box::new(123));
@@ -440,11 +439,7 @@ fn test_recursive_callback_panic() {
             // Produce a mutable reference
             let r = v.as_mut().unwrap();
             // Whoops, this will recurse into the function and produce another mutable reference!
-            lua.globals()
-                .get::<_, Function>("f")
-                .unwrap()
-                .call::<_, ()>(true)
-                .unwrap();
+            lua.globals().get::<_, Function>("f")?.call::<_, ()>(true)?;
             println!("Should not get here, mutable aliasing has occurred!");
             println!("value at {:p}", r as *mut _);
             println!("value is {}", r);
@@ -453,11 +448,13 @@ fn test_recursive_callback_panic() {
         Ok(())
     }).unwrap();
     lua.globals().set("f", f).unwrap();
+    assert!(
         lua.globals()
             .get::<_, Function>("f")
             .unwrap()
             .call::<_, ()>(false)
-        .unwrap();
+            .is_err()
+    )
 }
 
 #[test]
@@ -490,7 +487,8 @@ fn test_gc_error() {
         None,
     ) {
         Err(Error::GarbageCollectorError(_)) => {}
-        _ => panic!("__gc error did not result in correct type"),
+        Err(e) => panic!("__gc error did not result in correct error, instead: {}", e),
+        Ok(()) => panic!("__gc error did not result in error"),
     }
 }