Skip to content

Commit 36134e6

Browse files
kyrenkyren
committed
Userdata can have __gc metamethods called multiple times
Lua 5.3 has the ability for scripts to define __gc metamethods on tables, which gives them the ability to "resurrect" userdata after __gc has been called. This means, __gc can be called multiple times on userdata. This commit protects against this by simply panicking on access after resurrection. This is possibly not the best approach?
1 parent 396a4b0 commit 36134e6

File tree

3 files changed

+79
-44
lines changed

3 files changed

+79
-44
lines changed

‎src/lua.rs

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::ops::{Deref, DerefMut};
33
use std::iter::FromIterator;
44
use std::cell::{RefCell, Ref, RefMut};
55
use std::ptr;
6-
use std::mem;
76
use std::ffi::{CStr, CString};
87
use std::any::TypeId;
98
use std::marker::PhantomData;
@@ -909,9 +908,7 @@ impl<'lua> LuaUserData<'lua> {
909908
check_stack(lua.state, 3);
910909

911910
lua.push_ref(lua.state, &self.0);
912-
let userdata = ffi::lua_touserdata(lua.state, -1);
913911

914-
lua_assert!(lua.state, !userdata.is_null());
915912
lua_assert!(
916913
lua.state,
917914
ffi::lua_getmetatable(lua.state, -1) != 0,
@@ -928,7 +925,7 @@ impl<'lua> LuaUserData<'lua> {
928925
ffi::lua_pop(lua.state, 3);
929926
None
930927
} else {
931-
let res = func(&*(userdata as *const RefCell<T>));
928+
let res = func(&*get_userdata::<RefCell<T>>(lua.state, -3));
932929
ffi::lua_pop(lua.state, 3);
933930
Some(res)
934931
}
@@ -972,16 +969,12 @@ impl Lua {
972969
&LUA_USERDATA_REGISTRY_KEY as *const u8 as *mut c_void,
973970
);
974971

975-
let registered_userdata = ffi::lua_newuserdata(
976-
state,
977-
mem::size_of::<RefCell<HashMap<TypeId, c_int>>>(),
978-
) as *mut RefCell<HashMap<TypeId, c_int>>;
979-
ptr::write(registered_userdata, RefCell::new(HashMap::new()));
972+
push_userdata::<RefCell<HashMap<TypeId, c_int>>>(state, RefCell::new(HashMap::new()));
980973

981974
ffi::lua_newtable(state);
982975

983976
push_string(state, "__gc");
984-
ffi::lua_pushcfunction(state, destructor::<RefCell<HashMap<TypeId, c_int>>>);
977+
ffi::lua_pushcfunction(state, userdata_destructor::<RefCell<HashMap<TypeId, c_int>>>);
985978
ffi::lua_rawset(state, -3);
986979

987980
ffi::lua_setmetatable(state, -2);
@@ -998,7 +991,7 @@ impl Lua {
998991
ffi::lua_newtable(state);
999992

1000993
push_string(state, "__gc");
1001-
ffi::lua_pushcfunction(state, destructor::<LuaCallback>);
994+
ffi::lua_pushcfunction(state, userdata_destructor::<LuaCallback>);
1002995
ffi::lua_rawset(state, -3);
1003996

1004997
push_string(state, "__metatable");
@@ -1183,11 +1176,7 @@ impl Lua {
11831176
stack_guard(self.state, 0, move || {
11841177
check_stack(self.state, 2);
11851178

1186-
let data = RefCell::new(data);
1187-
let data_userdata =
1188-
ffi::lua_newuserdata(self.state, mem::size_of::<RefCell<T>>()) as
1189-
*mut RefCell<T>;
1190-
ptr::write(data_userdata, data);
1179+
push_userdata::<RefCell<T>>(self.state, RefCell::new(data));
11911180

11921181
ffi::lua_rawgeti(
11931182
self.state,
@@ -1322,8 +1311,7 @@ impl Lua {
13221311
ephemeral: true,
13231312
};
13241313

1325-
let func = &mut *(ffi::lua_touserdata(state, ffi::lua_upvalueindex(1)) as
1326-
*mut LuaCallback);
1314+
let func = &mut *get_userdata::<LuaCallback>(state, ffi::lua_upvalueindex(1));
13271315

13281316
let nargs = ffi::lua_gettop(state);
13291317
let mut args = LuaMultiValue::new();
@@ -1346,10 +1334,7 @@ impl Lua {
13461334
stack_guard(self.state, 0, move || {
13471335
check_stack(self.state, 2);
13481336

1349-
let func_userdata =
1350-
ffi::lua_newuserdata(self.state, mem::size_of::<LuaCallback>()) as
1351-
*mut LuaCallback;
1352-
ptr::write(func_userdata, func);
1337+
push_userdata::<LuaCallback>(self.state, func);
13531338

13541339
ffi::lua_pushlightuserdata(
13551340
self.state,
@@ -1522,8 +1507,7 @@ impl Lua {
15221507
&LUA_USERDATA_REGISTRY_KEY as *const u8 as *mut c_void,
15231508
);
15241509
ffi::lua_gettable(self.state, ffi::LUA_REGISTRYINDEX);
1525-
let registered_userdata = ffi::lua_touserdata(self.state, -1) as
1526-
*mut RefCell<HashMap<TypeId, c_int>>;
1510+
let registered_userdata = &mut *get_userdata::<RefCell<HashMap<TypeId, c_int>>>(self.state, -1);
15271511
let mut map = (*registered_userdata).borrow_mut();
15281512
ffi::lua_pop(self.state, 1);
15291513

@@ -1604,7 +1588,7 @@ impl Lua {
16041588
}
16051589

16061590
push_string(self.state, "__gc");
1607-
ffi::lua_pushcfunction(self.state, destructor::<RefCell<T>>);
1591+
ffi::lua_pushcfunction(self.state, userdata_destructor::<RefCell<T>>);
16081592
ffi::lua_rawset(self.state, -3);
16091593

16101594
push_string(self.state, "__metatable");

‎src/tests.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,3 +802,51 @@ fn test_num_conversion() {
802802
lua.exec::<()>("a = math.huge", None).unwrap();
803803
assert!(globals.get::<_, i64>("n").is_err());
804804
}
805+
806+
#[test]
807+
#[should_panic]
808+
fn test_expired_userdata() {
809+
struct Userdata {
810+
id: u8,
811+
}
812+
813+
impl Drop for Userdata {
814+
fn drop(&mut self) {
815+
println!("dropping {}", self.id);
816+
}
817+
}
818+
819+
impl LuaUserDataType for Userdata {
820+
fn add_methods(methods: &mut LuaUserDataMethods<Self>) {
821+
methods.add_method("access", |lua, this, _| {
822+
println!("accessing userdata {}", this.id);
823+
lua.pack(())
824+
});
825+
}
826+
}
827+
828+
let lua = Lua::new();
829+
{
830+
let globals = lua.globals();
831+
globals.set("userdata", Userdata { id: 123 }).unwrap();
832+
}
833+
834+
lua.eval::<()>(r#"
835+
local tbl = setmetatable({
836+
userdata = userdata
837+
}, { __gc = function(self)
838+
-- resurrect userdata
839+
hatch = self.userdata
840+
end })
841+
842+
print("userdata = ", userdata)
843+
print("hatch = ", hatch)
844+
print "collecting..."
845+
tbl = nil
846+
userdata = nil -- make table and userdata collectable
847+
collectgarbage("collect")
848+
print("userdata = ", userdata)
849+
print("hatch = ", hatch)
850+
hatch:access()
851+
"#, None).unwrap();
852+
}

‎src/util.rs

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,7 @@ pub unsafe fn handle_error(state: *mut ffi::lua_State, err: c_int) -> LuaResult<
213213
Err(err)
214214

215215
} else if is_wrapped_panic(state, -1) {
216-
let userdata = ffi::lua_touserdata(state, -1);
217-
let panic = &mut *(userdata as *mut WrappedPanic);
216+
let panic = &mut *get_userdata::<WrappedPanic>(state, -1);
218217
if let Some(p) = panic.0.take() {
219218
ffi::lua_settop(state, 0);
220219
resume_unwind(p);
@@ -275,10 +274,22 @@ pub unsafe fn push_string(state: *mut ffi::lua_State, s: &str) {
275274
ffi::lua_pushlstring(state, s.as_ptr() as *const c_char, s.len());
276275
}
277276

278-
pub unsafe extern "C" fn destructor<T>(state: *mut ffi::lua_State) -> c_int {
277+
pub unsafe fn push_userdata<T>(state: *mut ffi::lua_State, t: T) -> *mut T {
278+
let ud = ffi::lua_newuserdata(state, mem::size_of::<Option<T>>()) as *mut Option<T>;
279+
ptr::write(ud, None);
280+
*ud = Some(t);
281+
(*ud).as_mut().unwrap()
282+
}
283+
284+
pub unsafe fn get_userdata<T>(state: *mut ffi::lua_State, index: c_int) -> *mut T {
285+
let ud = ffi::lua_touserdata(state, index) as *mut Option<T>;
286+
lua_assert!(state, !ud.is_null());
287+
(*ud).as_mut().expect("access of expired userdata")
288+
}
289+
290+
pub unsafe extern "C" fn userdata_destructor<T>(state: *mut ffi::lua_State) -> c_int {
279291
match catch_unwind(|| {
280-
let obj = &mut *(ffi::lua_touserdata(state, 1) as *mut T);
281-
mem::replace(obj, mem::uninitialized());
292+
*(ffi::lua_touserdata(state, 1) as *mut Option<T>) = None;
282293
0
283294
}) {
284295
Ok(r) => r,
@@ -435,8 +446,7 @@ pub struct WrappedPanic(pub Option<Box<Any + Send>>);
435446
pub unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: LuaError) {
436447
unsafe extern "C" fn error_tostring(state: *mut ffi::lua_State) -> c_int {
437448
callback_error(state, || if is_wrapped_error(state, -1) {
438-
let userdata = ffi::lua_touserdata(state, -1);
439-
let error = &*(userdata as *const WrappedError);
449+
let error = &*get_userdata::<WrappedError>(state, -1);
440450
push_string(state, &error.0.to_string());
441451
ffi::lua_remove(state, -2);
442452

@@ -452,10 +462,7 @@ pub unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: LuaError) {
452462

453463
ffi::luaL_checkstack(state, 2, ptr::null());
454464

455-
let err_userdata = ffi::lua_newuserdata(state, mem::size_of::<WrappedError>()) as
456-
*mut WrappedError;
457-
458-
ptr::write(err_userdata, WrappedError(err));
465+
push_userdata(state, WrappedError(err));
459466

460467
get_error_metatable(state);
461468
if ffi::lua_isnil(state, -1) != 0 {
@@ -471,7 +478,7 @@ pub unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: LuaError) {
471478
ffi::lua_pushvalue(state, -2);
472479

473480
push_string(state, "__gc");
474-
ffi::lua_pushcfunction(state, destructor::<WrappedError>);
481+
ffi::lua_pushcfunction(state, userdata_destructor::<WrappedError>);
475482
ffi::lua_settable(state, -3);
476483

477484
push_string(state, "__tostring");
@@ -492,10 +499,7 @@ pub unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: LuaError) {
492499
pub unsafe fn push_wrapped_panic(state: *mut ffi::lua_State, panic: Box<Any + Send>) {
493500
ffi::luaL_checkstack(state, 2, ptr::null());
494501

495-
let panic_userdata = ffi::lua_newuserdata(state, mem::size_of::<WrappedPanic>()) as
496-
*mut WrappedPanic;
497-
498-
ptr::write(panic_userdata, WrappedPanic(Some(panic)));
502+
push_userdata(state, WrappedPanic(Some(panic)));
499503

500504
get_panic_metatable(state);
501505
if ffi::lua_isnil(state, -1) != 0 {
@@ -511,7 +515,7 @@ pub unsafe fn push_wrapped_panic(state: *mut ffi::lua_State, panic: Box<Any + Se
511515
ffi::lua_pushvalue(state, -2);
512516

513517
push_string(state, "__gc");
514-
ffi::lua_pushcfunction(state, destructor::<WrappedPanic>);
518+
ffi::lua_pushcfunction(state, userdata_destructor::<WrappedPanic>);
515519
ffi::lua_settable(state, -3);
516520

517521
push_string(state, "__metatable");
@@ -530,8 +534,7 @@ pub unsafe fn pop_wrapped_error(state: *mut ffi::lua_State) -> Option<LuaError>
530534
if ffi::lua_gettop(state) == 0 || !is_wrapped_error(state, -1) {
531535
None
532536
} else {
533-
let userdata = ffi::lua_touserdata(state, -1);
534-
let err = &*(userdata as *const WrappedError);
537+
let err = &*get_userdata::<WrappedError>(state, -1);
535538
let err = err.0.clone();
536539
ffi::lua_pop(state, 1);
537540
Some(err)

0 commit comments

Comments
 (0)