summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorkyren <kerriganw@gmail.com>2017-12-04 00:35:13 -0500
committerkyren <kerriganw@gmail.com>2017-12-04 00:35:13 -0500
commit51838f3509326d17dd5cb692f54839cccc764e35 (patch)
treefa30b615cbf52e177040e673a45d6e5670573b2d
parentd76935e68381c990f96d3a36d9640713c7165941 (diff)
downloadmlua-51838f3509326d17dd5cb692f54839cccc764e35.zip
Include garbage collector error type, remove unnecessary setmetatable wrapper
-rw-r--r--src/error.rs10
-rw-r--r--src/lua.rs11
-rw-r--r--src/table.rs29
-rw-r--r--src/tests.rs22
-rw-r--r--src/util.rs57
5 files changed, 44 insertions, 85 deletions
diff --git a/src/error.rs b/src/error.rs
index 04e909a..a509432 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -3,7 +3,7 @@ use std::sync::Arc;
use std::error::Error as StdError;
use std::result::Result as StdResult;
-/// Error type returned by rlua methods.
+/// Error type returned by `rlua` methods.
#[derive(Debug, Clone)]
pub enum Error {
/// Syntax error while parsing Lua source code.
@@ -22,6 +22,10 @@ pub enum Error {
/// Among other things, this includes invoking operators on wrong types (such as calling or
/// indexing a `nil` value).
RuntimeError(String),
+ /// Lua garbage collector error, aka `LUA_ERRGCMM`.
+ ///
+ /// The Lua VM returns this error when there is an error running a `__gc` metamethod.
+ GarbageCollectorError(String),
/// A Rust value could not be converted to a Lua value.
ToLuaConversionError {
/// Name of the Rust type that could not be converted.
@@ -95,7 +99,7 @@ pub enum Error {
ExternalError(Arc<StdError + Send + Sync>),
}
-/// A specialized `Result` type used by rlua's API.
+/// A specialized `Result` type used by `rlua`'s API.
pub type Result<T> = StdResult<T, Error>;
impl fmt::Display for Error {
@@ -103,6 +107,7 @@ impl fmt::Display for Error {
match *self {
Error::SyntaxError { ref message, .. } => write!(fmt, "syntax error: {}", message),
Error::RuntimeError(ref msg) => write!(fmt, "runtime error: {}", msg),
+ Error::GarbageCollectorError(ref msg) => write!(fmt, "garbage collector error: {}", msg),
Error::ToLuaConversionError {
from,
to,
@@ -142,6 +147,7 @@ impl StdError for Error {
match *self {
Error::SyntaxError { .. } => "syntax error",
Error::RuntimeError(_) => "runtime error",
+ Error::GarbageCollectorError(_) => "garbage collector error",
Error::ToLuaConversionError { .. } => "conversion error to lua",
Error::FromLuaConversionError { .. } => "conversion error from lua",
Error::CoroutineInactive => "attempt to resume inactive coroutine",
diff --git a/src/lua.rs b/src/lua.rs
index 5e5c048..471f717 100644
--- a/src/lua.rs
+++ b/src/lua.rs
@@ -1110,8 +1110,11 @@ impl Lua {
} else {
let p = libc::realloc(ptr as *mut libc::c_void, nsize);
if p.is_null() {
- // We must abort on OOM, because otherwise this will result in an unsafe
- // longjmp.
+ // We require that OOM results in an abort, and that the lua allocator function
+ // never errors. Since this is what rust itself normally does on OOM, this is
+ // not really a huge loss. Importantly, this allows us to turn off the gc, and
+ // then know that calling Lua API functions marked as 'm' will not result in a
+ // 'longjmp' error while the gc is off.
eprintln!("Out of memory in Lua allocation, aborting!");
process::abort()
} else {
@@ -1193,10 +1196,6 @@ impl Lua {
ffi::lua_pushcfunction(state, safe_xpcall);
ffi::lua_rawset(state, -3);
- push_string(state, "setmetatable").unwrap();
- ffi::lua_pushcfunction(state, safe_setmetatable);
- ffi::lua_rawset(state, -3);
-
ffi::lua_pop(state, 1);
});
diff --git a/src/table.rs b/src/table.rs
index 7347b7f..fa2d506 100644
--- a/src/table.rs
+++ b/src/table.rs
@@ -435,7 +435,7 @@ where
#[cfg(test)]
mod tests {
use super::Table;
- use error::Result;
+ use error::{Result};
use lua::{Lua, Nil, Value};
#[test]
@@ -559,33 +559,6 @@ mod tests {
}
#[test]
- fn test_setmetatable_gc() {
- let lua = Lua::new();
- lua.exec::<()>(
- r#"
- val = nil
- table = {}
- setmetatable(table, {
- __gc = function()
- val = "gcwascalled"
- end
- })
- table_badgc = {}
- setmetatable(table_badgc, {
- __gc = "what's a gc"
- })
- table = nil
- table_badgc = nil
- collectgarbage("collect")
- "#,
- None,
- ).unwrap();
-
- let globals = lua.globals();
- assert_eq!(globals.get::<_, String>("val").unwrap(), "gcwascalled");
- }
-
- #[test]
fn test_metatable() {
let lua = Lua::new();
diff --git a/src/tests.rs b/src/tests.rs
index 56b8040..a1c82df 100644
--- a/src/tests.rs
+++ b/src/tests.rs
@@ -646,6 +646,28 @@ fn test_set_metatable_nil() {
).unwrap();
}
+#[test]
+fn test_gc_error() {
+ let lua = Lua::new();
+ match lua.exec::<()>(
+ r#"
+ val = nil
+ table = {}
+ setmetatable(table, {
+ __gc = function()
+ error("gcwascalled")
+ end
+ })
+ table = nil
+ collectgarbage("collect")
+ "#,
+ None,
+ ) {
+ Err(Error::GarbageCollectorError(_)) => {}
+ _ => panic!("__gc error did not result in correct type"),
+ }
+}
+
// TODO: Need to use compiletest-rs or similar to make sure these don't compile.
/*
#[test]
diff --git a/src/util.rs b/src/util.rs
index 2f4a363..3507cc4 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -201,15 +201,11 @@ pub unsafe fn pop_error(state: *mut ffi::lua_State, err_code: c_int) -> Error {
ffi::LUA_ERRMEM => {
// This should be impossible, as we set the lua allocator to one that aborts
// instead of failing.
- eprintln!("Lua memory error, aborting!");
+ eprintln!("impossible Lua allocation error, aborting!");
process::abort()
}
ffi::LUA_ERRGCMM => {
- // This should be impossible, since we wrap setmetatable to protect __gc
- // metamethods, but if we do end up here then the same logic as setmetatable
- // applies and we must abort.
- eprintln!("Lua error during __gc, aborting!");
- process::abort()
+ Error::GarbageCollectorError(err_string)
}
_ => lua_panic!(state, "internal error: unrecognized lua error code"),
}
@@ -354,43 +350,6 @@ pub unsafe extern "C" fn safe_xpcall(state: *mut ffi::lua_State) -> c_int {
}
}
-// Safely call setmetatable, if a __gc function is given, will wrap it in pcall, and panic on error.
-pub unsafe extern "C" fn safe_setmetatable(state: *mut ffi::lua_State) -> c_int {
- if ffi::lua_gettop(state) < 2 {
- ffi::lua_pushstring(state, cstr!("not enough arguments to setmetatable"));
- ffi::lua_error(state);
- }
-
- // Wrapping the __gc method in setmetatable ONLY works because Lua 5.3 only honors the __gc
- // method when it exists upon calling setmetatable, and ignores it if it is set later.
- ffi::lua_pushstring(state, cstr!("__gc"));
- if ffi::lua_istable(state, -2) == 1 && ffi::lua_rawget(state, -2) == ffi::LUA_TFUNCTION {
- unsafe extern "C" fn safe_gc(state: *mut ffi::lua_State) -> c_int {
- ffi::lua_pushvalue(state, ffi::lua_upvalueindex(1));
- ffi::lua_insert(state, 1);
- if ffi::lua_pcall(state, 1, 0, 0) != ffi::LUA_OK {
- // If a user supplied __gc metamethod causes an error, we must always abort. We may
- // be inside a protected context due to being in a callback, but inside an
- // unprotected ffi call that can cause memory errors, so may be at risk of
- // longjmping over arbitrary rust.
- eprintln!("Lua error during __gc, aborting!");
- process::abort()
- } else {
- ffi::lua_gettop(state)
- }
- }
-
- ffi::lua_pushcclosure(state, safe_gc, 1);
- ffi::lua_pushstring(state, cstr!("__gc"));
- ffi::lua_insert(state, -2);
- ffi::lua_rawset(state, -3);
- } else {
- ffi::lua_pop(state, 1);
- }
- ffi::lua_setmetatable(state, -2);
- 1
-}
-
// Does not call checkstack, uses 1 stack space
pub unsafe fn main_state(state: *mut ffi::lua_State) -> *mut ffi::lua_State {
ffi::lua_rawgeti(state, ffi::LUA_REGISTRYINDEX, ffi::LUA_RIDX_MAINTHREAD);
@@ -593,12 +552,12 @@ unsafe fn get_panic_metatable(state: *mut ffi::lua_State) -> c_int {
ffi::LUA_TTABLE
}
-// Runs the given function with the Lua garbage collector disabled. `rlua` assumes that all memory
-// errors are aborts, so in this way, 'm' functions that may also cause a `__gc` metamethod error
-// are guaranteed not to cause a Lua error (longjmp). The given function should never panic or
-// longjmp, because this could inadverntently disable the gc. This is useful when error handling
-// must allocate, and `__gc` errors at that time would shadow more important errors, or be extremely
-// difficult to handle safely.
+// Runs the given function with the Lua garbage collector disabled. `rlua` assumes that all
+// allocation failures are aborts, so when the garbage collector is disabled, 'm' functions that can
+// cause either an allocation error or a a `__gc` metamethod error are prevented from causing errors
+// at all. The given function should never panic or longjmp, because this could inadverntently
+// disable the gc. This is useful when error handling must allocate, and `__gc` errors at that time
+// would shadow more important errors, or be extremely difficult to handle safely.
unsafe fn gc_guard<R, F: FnOnce() -> R>(state: *mut ffi::lua_State, f: F) -> R {
if ffi::lua_gc(state, ffi::LUA_GCISRUNNING, 0) != 0 {
ffi::lua_gc(state, ffi::LUA_GCSTOP, 0);