Skip to content

Commit d331e4b

Browse files
kyrenkyren
committed
Error correctly on too many arguments / returns / binds / recursions
There are also some other drive-by changes to fix panicking in extern "C" functions and other edge case stack errors
1 parent fe6e4bd commit d331e4b

File tree

9 files changed

+186
-64
lines changed

9 files changed

+186
-64
lines changed

‎src/error.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,25 @@ pub enum Error {
2626
///
2727
/// The Lua VM returns this error when there is an error running a `__gc` metamethod.
2828
GarbageCollectorError(String),
29-
/// A callback has triggered Lua code that has called the same callback again.
29+
/// A mutable callback has triggered Lua code that has called the same mutable callback again.
3030
///
31-
/// This is an error because `rlua` callbacks are FnMut and thus can only be mutably borrowed
32-
/// once.
33-
RecursiveCallback,
31+
/// This is an error because a mutable callback can only be borrowed mutably once.
32+
RecursiveMutCallback,
3433
/// Either a callback or a userdata method has been called, but the callback or userdata has
3534
/// been destructed.
3635
///
3736
/// This can happen either due to to being destructed in a previous __gc, or due to being
3837
/// destructed from exiting a `Lua::scope` call.
3938
CallbackDestructed,
39+
/// Not enough stack space to place arguments to Lua functions or return values from callbacks.
40+
///
41+
/// Due to the way `rlua` works, it should not be directly possible to run out of stack space
42+
/// during normal use. The only way that this error can be triggered is if a `Function` is
43+
/// called with a huge number of arguments, or a rust callback returns a huge number of return
44+
/// values.
45+
StackError,
46+
/// Too many arguments to `Function::bind`
47+
BindError,
4048
/// A Rust value could not be converted to a Lua value.
4149
ToLuaConversionError {
4250
/// Name of the Rust type that could not be converted.
@@ -123,11 +131,19 @@ impl fmt::Display for Error {
123131
Error::GarbageCollectorError(ref msg) => {
124132
write!(fmt, "garbage collector error: {}", msg)
125133
}
126-
Error::RecursiveCallback => write!(fmt, "callback called recursively"),
134+
Error::RecursiveMutCallback => write!(fmt, "mutable callback called recursively"),
127135
Error::CallbackDestructed => write!(
128136
fmt,
129137
"a destructed callback or destructed userdata method was called"
130138
),
139+
Error::StackError => write!(
140+
fmt,
141+
"out of Lua stack, too many arguments to a Lua function or too many return values from a callback"
142+
),
143+
Error::BindError => write!(
144+
fmt,
145+
"too many arguments to Function::bind"
146+
),
131147
Error::ToLuaConversionError {
132148
from,
133149
to,

‎src/function.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::ptr;
12
use std::os::raw::c_int;
23

34
use ffi;
@@ -65,7 +66,7 @@ impl<'lua> Function<'lua> {
6566
stack_err_guard(lua.state, 0, || {
6667
let args = args.to_lua_multi(lua)?;
6768
let nargs = args.len() as c_int;
68-
check_stack(lua.state, nargs + 3);
69+
check_stack_err(lua.state, nargs + 3)?;
6970

7071
ffi::lua_pushcfunction(lua.state, error_traceback);
7172
let stack_start = ffi::lua_gettop(lua.state);
@@ -124,7 +125,7 @@ impl<'lua> Function<'lua> {
124125
unsafe extern "C" fn bind_call_impl(state: *mut ffi::lua_State) -> c_int {
125126
let nargs = ffi::lua_gettop(state);
126127
let nbinds = ffi::lua_tointeger(state, ffi::lua_upvalueindex(2)) as c_int;
127-
check_stack(state, nbinds + 2);
128+
ffi::luaL_checkstack(state, nbinds + 2, ptr::null());
128129

129130
ffi::lua_settop(state, nargs + nbinds + 1);
130131
ffi::lua_rotate(state, -(nargs + nbinds + 1), nbinds + 1);
@@ -144,10 +145,16 @@ impl<'lua> Function<'lua> {
144145
let lua = self.0.lua;
145146
unsafe {
146147
stack_err_guard(lua.state, 0, || {
148+
const MAX_LUA_UPVALUES: c_int = 255;
149+
147150
let args = args.to_lua_multi(lua)?;
148151
let nargs = args.len() as c_int;
149152

150-
check_stack(lua.state, nargs + 3);
153+
if nargs > MAX_LUA_UPVALUES {
154+
return Err(Error::BindError);
155+
}
156+
157+
check_stack_err(lua.state, nargs + 3)?;
151158
lua.push_ref(lua.state, &self.0);
152159
ffi::lua_pushinteger(lua.state, nargs as ffi::lua_Integer);
153160
for arg in args {

‎src/lua.rs

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::{mem, process, ptr, str};
22
use std::sync::{Arc, Mutex};
3-
use std::ops::DerefMut;
43
use std::cell::RefCell;
54
use std::ffi::CString;
65
use std::any::{Any, TypeId};
@@ -32,7 +31,7 @@ pub struct Lua {
3231
/// !Send, and callbacks that are !Send and not 'static.
3332
pub struct Scope<'lua, 'scope> {
3433
lua: &'lua Lua,
35-
destructors: RefCell<Vec<Box<FnMut(*mut ffi::lua_State) -> Box<Any>>>>,
34+
destructors: RefCell<Vec<Box<Fn(*mut ffi::lua_State) -> Box<Any>>>>,
3635
// 'scope lifetime must be invariant
3736
_scope: PhantomData<&'scope mut &'scope ()>,
3837
}
@@ -265,20 +264,33 @@ impl Lua {
265264
///
266265
/// [`ToLua`]: trait.ToLua.html
267266
/// [`ToLuaMulti`]: trait.ToLuaMulti.html
268-
pub fn create_function<'lua, 'callback, A, R, F>(
269-
&'lua self,
270-
mut func: F,
271-
) -> Result<Function<'lua>>
267+
pub fn create_function<'lua, 'callback, A, R, F>(&'lua self, func: F) -> Result<Function<'lua>>
272268
where
273269
A: FromLuaMulti<'callback>,
274270
R: ToLuaMulti<'callback>,
275-
F: 'static + Send + FnMut(&'callback Lua, A) -> Result<R>,
271+
F: 'static + Send + Fn(&'callback Lua, A) -> Result<R>,
276272
{
277273
self.create_callback_function(Box::new(move |lua, args| {
278274
func(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
279275
}))
280276
}
281277

278+
pub fn create_function_mut<'lua, 'callback, A, R, F>(
279+
&'lua self,
280+
func: F,
281+
) -> Result<Function<'lua>>
282+
where
283+
A: FromLuaMulti<'callback>,
284+
R: ToLuaMulti<'callback>,
285+
F: 'static + Send + FnMut(&'callback Lua, A) -> Result<R>,
286+
{
287+
let func = RefCell::new(func);
288+
self.create_function(move |lua, args| {
289+
(&mut *func.try_borrow_mut()
290+
.map_err(|_| Error::RecursiveMutCallback)?)(lua, args)
291+
})
292+
}
293+
282294
/// Wraps a Lua function into a new thread (or coroutine).
283295
///
284296
/// Equivalent to `coroutine.create`.
@@ -354,7 +366,7 @@ impl Lua {
354366
Value::String(s) => Ok(s),
355367
v => unsafe {
356368
stack_err_guard(self.state, 0, || {
357-
check_stack(self.state, 2);
369+
check_stack(self.state, 4);
358370
let ty = v.type_name();
359371
self.push_value(self.state, v);
360372
let s =
@@ -383,7 +395,7 @@ impl Lua {
383395
Value::Integer(i) => Ok(i),
384396
v => unsafe {
385397
stack_guard(self.state, 0, || {
386-
check_stack(self.state, 1);
398+
check_stack(self.state, 2);
387399
let ty = v.type_name();
388400
self.push_value(self.state, v);
389401
let mut isint = 0;
@@ -412,7 +424,7 @@ impl Lua {
412424
Value::Number(n) => Ok(n),
413425
v => unsafe {
414426
stack_guard(self.state, 0, || {
415-
check_stack(self.state, 1);
427+
check_stack(self.state, 2);
416428
let ty = v.type_name();
417429
self.push_value(self.state, v);
418430
let mut isnum = 0;
@@ -511,7 +523,7 @@ impl Lua {
511523
pub fn create_registry_value<'lua, T: ToLua<'lua>>(&'lua self, t: T) -> Result<RegistryKey> {
512524
unsafe {
513525
stack_guard(self.state, 0, || {
514-
check_stack(self.state, 1);
526+
check_stack(self.state, 2);
515527

516528
self.push_value(self.state, t.to_lua(self)?);
517529
let registry_id = gc_guard(self.state, || {
@@ -592,7 +604,7 @@ impl Lua {
592604
}
593605
}
594606

595-
// Uses 1 stack space, does not call checkstack
607+
// Uses 2 stack spaces, does not call checkstack
596608
pub(crate) unsafe fn push_value(&self, state: *mut ffi::lua_State, value: Value) {
597609
match value {
598610
Value::Nil => {
@@ -730,7 +742,7 @@ impl Lua {
730742
// Used if both an __index metamethod is set and regular methods, checks methods table
731743
// first, then __index metamethod.
732744
unsafe extern "C" fn meta_index_impl(state: *mut ffi::lua_State) -> c_int {
733-
check_stack(state, 2);
745+
ffi::luaL_checkstack(state, 2, ptr::null());
734746

735747
ffi::lua_pushvalue(state, -1);
736748
ffi::lua_gettable(state, ffi::lua_upvalueindex(1));
@@ -922,7 +934,7 @@ impl Lua {
922934
ffi::lua_newtable(state);
923935

924936
push_string(state, "__gc").unwrap();
925-
ffi::lua_pushcfunction(state, userdata_destructor::<RefCell<Callback>>);
937+
ffi::lua_pushcfunction(state, userdata_destructor::<Callback>);
926938
ffi::lua_rawset(state, -3);
927939

928940
push_string(state, "__metatable").unwrap();
@@ -977,10 +989,7 @@ impl Lua {
977989
return Err(Error::CallbackDestructed);
978990
}
979991

980-
let func = get_userdata::<RefCell<Callback>>(state, ffi::lua_upvalueindex(1));
981-
let mut func = (*func)
982-
.try_borrow_mut()
983-
.map_err(|_| Error::RecursiveCallback)?;
992+
let func = get_userdata::<Callback>(state, ffi::lua_upvalueindex(1));
984993

985994
let nargs = ffi::lua_gettop(state);
986995
let mut args = MultiValue::new();
@@ -989,10 +998,10 @@ impl Lua {
989998
args.push_front(lua.pop_value(state));
990999
}
9911000

992-
let results = func.deref_mut()(&lua, args)?;
1001+
let results = (*func)(&lua, args)?;
9931002
let nresults = results.len() as c_int;
9941003

995-
check_stack(state, nresults);
1004+
check_stack_err(state, nresults)?;
9961005

9971006
for r in results {
9981007
lua.push_value(state, r);
@@ -1006,7 +1015,7 @@ impl Lua {
10061015
stack_err_guard(self.state, 0, move || {
10071016
check_stack(self.state, 2);
10081017

1009-
push_userdata::<RefCell<Callback>>(self.state, RefCell::new(func))?;
1018+
push_userdata::<Callback>(self.state, func)?;
10101019

10111020
ffi::lua_pushlightuserdata(
10121021
self.state,
@@ -1053,15 +1062,15 @@ impl Lua {
10531062
}
10541063

10551064
impl<'lua, 'scope> Scope<'lua, 'scope> {
1056-
pub fn create_function<'callback, A, R, F>(&self, mut func: F) -> Result<Function<'lua>>
1065+
pub fn create_function<'callback, A, R, F>(&self, func: F) -> Result<Function<'lua>>
10571066
where
10581067
A: FromLuaMulti<'callback>,
10591068
R: ToLuaMulti<'callback>,
1060-
F: 'scope + FnMut(&'callback Lua, A) -> Result<R>,
1069+
F: 'scope + Fn(&'callback Lua, A) -> Result<R>,
10611070
{
10621071
unsafe {
10631072
let f: Box<
1064-
FnMut(&'callback Lua, MultiValue<'callback>) -> Result<MultiValue<'callback>>,
1073+
Fn(&'callback Lua, MultiValue<'callback>) -> Result<MultiValue<'callback>>,
10651074
> = Box::new(move |lua, args| {
10661075
func(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
10671076
});
@@ -1082,7 +1091,7 @@ impl<'lua, 'scope> Scope<'lua, 'scope> {
10821091
ffi::luaL_unref(state, ffi::LUA_REGISTRYINDEX, registry_id);
10831092

10841093
ffi::lua_getupvalue(state, -1, 1);
1085-
let ud = take_userdata::<RefCell<Callback>>(state);
1094+
let ud = take_userdata::<Callback>(state);
10861095

10871096
ffi::lua_pushnil(state);
10881097
ffi::lua_setupvalue(state, -2, 1);
@@ -1094,6 +1103,19 @@ impl<'lua, 'scope> Scope<'lua, 'scope> {
10941103
}
10951104
}
10961105

1106+
pub fn create_function_mut<'callback, A, R, F>(&self, func: F) -> Result<Function<'lua>>
1107+
where
1108+
A: FromLuaMulti<'callback>,
1109+
R: ToLuaMulti<'callback>,
1110+
F: 'scope + FnMut(&'callback Lua, A) -> Result<R>,
1111+
{
1112+
let func = RefCell::new(func);
1113+
self.create_function(move |lua, args| {
1114+
(&mut *func.try_borrow_mut()
1115+
.map_err(|_| Error::RecursiveMutCallback)?)(lua, args)
1116+
})
1117+
}
1118+
10971119
pub fn create_userdata<T>(&self, data: T) -> Result<AnyUserData<'lua>>
10981120
where
10991121
T: UserData,
@@ -1128,7 +1150,7 @@ impl<'lua, 'scope> Drop for Scope<'lua, 'scope> {
11281150
let to_drop = self.destructors
11291151
.get_mut()
11301152
.drain(..)
1131-
.map(|mut destructor| destructor(state))
1153+
.map(|destructor| destructor(state))
11321154
.collect::<Vec<_>>();
11331155
drop(to_drop);
11341156
}

‎src/table.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ impl<'lua> Table<'lua> {
125125
let lua = self.0.lua;
126126
unsafe {
127127
stack_err_guard(lua.state, 0, || {
128-
check_stack(lua.state, 3);
128+
check_stack(lua.state, 6);
129129
lua.push_ref(lua.state, &self.0);
130130
lua.push_value(lua.state, key.to_lua(lua)?);
131131
lua.push_value(lua.state, value.to_lua(lua)?);
@@ -142,7 +142,7 @@ impl<'lua> Table<'lua> {
142142
let lua = self.0.lua;
143143
unsafe {
144144
stack_err_guard(lua.state, 0, || {
145-
check_stack(lua.state, 2);
145+
check_stack(lua.state, 3);
146146
lua.push_ref(lua.state, &self.0);
147147
lua.push_value(lua.state, key.to_lua(lua)?);
148148
ffi::lua_rawget(lua.state, -2);

0 commit comments

Comments
 (0)