From 3ae7e73b1808e95ed23fca5e03767009308f4373 Mon Sep 17 00:00:00 2001 From: Michael Pankov Date: Sat, 15 Apr 2017 17:37:39 +0300 Subject: [PATCH 1/6] Drop closure environment --- hlua/src/functions_write.rs | 65 +++++++++++++++++++++++++++++++++++++ hlua/src/userdata.rs | 2 +- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/hlua/src/functions_write.rs b/hlua/src/functions_write.rs index eda775a..0f04d72 100644 --- a/hlua/src/functions_write.rs +++ b/hlua/src/functions_write.rs @@ -198,6 +198,27 @@ macro_rules! impl_function_ext { let lua_data: *mut Z = mem::transmute(lua_data); ptr::write(lua_data, self.function); + let lua_raw = lua.as_mut_lua(); + + // Creating a metatable. + ffi::lua_newtable(lua.as_mut_lua().0); + + // Index "__gc" in the metatable calls the object's destructor. + + // TODO: Could use std::intrinsics::needs_drop to avoid that if not needed. + // After some discussion on IRC, it would be acceptable to add a reexport in libcore + // without going through the RFC process. + { + match "__gc".push_to_lua(&mut lua) { + Ok(p) => p.forget(), + Err(_) => unreachable!(), + }; + + ffi::lua_pushcfunction(lua.as_mut_lua().0, ::userdata::destructor_wrapper::); + ffi::lua_settable(lua.as_mut_lua().0, -3); + } + ffi::lua_setmetatable(lua_raw.0, -2); + // pushing wrapper as a closure let wrapper: extern fn(*mut ffi::lua_State) -> libc::c_int = wrapper::; ffi::lua_pushcclosure(lua.as_mut_lua().0, wrapper, 1); @@ -244,6 +265,27 @@ macro_rules! impl_function_ext { let lua_data: *mut Z = mem::transmute(lua_data); ptr::write(lua_data, self.function); + let lua_raw = lua.as_mut_lua(); + + // Creating a metatable. + ffi::lua_newtable(lua.as_mut_lua().0); + + // Index "__gc" in the metatable calls the object's destructor. + + // TODO: Could use std::intrinsics::needs_drop to avoid that if not needed. + // After some discussion on IRC, it would be acceptable to add a reexport in libcore + // without going through the RFC process. + { + match "__gc".push_to_lua(&mut lua) { + Ok(p) => p.forget(), + Err(_) => unreachable!(), + }; + + ffi::lua_pushcfunction(lua.as_mut_lua().0, ::userdata::destructor_wrapper::); + ffi::lua_settable(lua.as_mut_lua().0, -3); + } + ffi::lua_setmetatable(lua_raw.0, -2); + // pushing wrapper as a closure let wrapper: extern fn(*mut ffi::lua_State) -> libc::c_int = wrapper::; ffi::lua_pushcclosure(lua.as_mut_lua().0, wrapper, 1); @@ -496,4 +538,27 @@ mod tests { assert_eq!(a, 20) } + // FIXME: This test is wrong. + // It passes before PR, and + // https://github.com/mkpankov/hlua-test/blob/master/src/main.rs + // which it is based on does not. + // I'm out of ideas who calls Drop for Foo here, in test env. + #[test] + #[should_panic(expected = "Destructor for Foo is run")] + fn closures_drop_env() { + #[derive(Debug)] + struct Foo { }; + impl Drop for Foo { + fn drop(&mut self) { + panic!("Destructor for Foo is run"); + } + } + let foo = Foo { }; + + { + let mut lua = Lua::new(); + + lua.set("print_foo", function0(|| println!("{:?}", foo))); + } + } } diff --git a/hlua/src/userdata.rs b/hlua/src/userdata.rs index 799c889..0253899 100644 --- a/hlua/src/userdata.rs +++ b/hlua/src/userdata.rs @@ -18,7 +18,7 @@ use LuaTable; // Called when an object inside Lua is being dropped. #[inline] -extern "C" fn destructor_wrapper(lua: *mut ffi::lua_State) -> libc::c_int { +pub extern "C" fn destructor_wrapper(lua: *mut ffi::lua_State) -> libc::c_int { unsafe { let obj = ffi::lua_touserdata(lua, -1); ptr::drop_in_place(obj as *mut TypeId); From c285db538fae6d5e6a3b8937c0fd871976eb3fed Mon Sep 17 00:00:00 2001 From: Michael Pankov Date: Sat, 15 Apr 2017 18:20:40 +0300 Subject: [PATCH 2/6] Fix the test Forgot the move --- hlua/src/functions_write.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/hlua/src/functions_write.rs b/hlua/src/functions_write.rs index 0f04d72..65f4b47 100644 --- a/hlua/src/functions_write.rs +++ b/hlua/src/functions_write.rs @@ -538,11 +538,6 @@ mod tests { assert_eq!(a, 20) } - // FIXME: This test is wrong. - // It passes before PR, and - // https://github.com/mkpankov/hlua-test/blob/master/src/main.rs - // which it is based on does not. - // I'm out of ideas who calls Drop for Foo here, in test env. #[test] #[should_panic(expected = "Destructor for Foo is run")] fn closures_drop_env() { @@ -558,7 +553,7 @@ mod tests { { let mut lua = Lua::new(); - lua.set("print_foo", function0(|| println!("{:?}", foo))); + lua.set("print_foo", function0(move || println!("{:?}", foo))); } } } From 9063d00579ffad425d9f4f2a7780685cd33e0d36 Mon Sep 17 00:00:00 2001 From: Michael Pankov Date: Sat, 15 Apr 2017 18:24:10 +0300 Subject: [PATCH 3/6] Don't panic in test --- hlua/src/functions_write.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/hlua/src/functions_write.rs b/hlua/src/functions_write.rs index 65f4b47..c7e6af9 100644 --- a/hlua/src/functions_write.rs +++ b/hlua/src/functions_write.rs @@ -539,21 +539,27 @@ mod tests { } #[test] - #[should_panic(expected = "Destructor for Foo is run")] fn closures_drop_env() { + static mut DID_DESTRUCTOR_RUN: bool = false; + #[derive(Debug)] struct Foo { }; impl Drop for Foo { fn drop(&mut self) { - panic!("Destructor for Foo is run"); + unsafe { + DID_DESTRUCTOR_RUN = true; + } } } - let foo = Foo { }; - { - let mut lua = Lua::new(); + let foo = Foo { }; - lua.set("print_foo", function0(move || println!("{:?}", foo))); + { + let mut lua = Lua::new(); + + lua.set("print_foo", function0(move || println!("{:?}", foo))); + } } + assert_eq!(unsafe { DID_DESTRUCTOR_RUN }, true); } } From 6b3fdeeaaa16f2319de7f08040cddde3f86de005 Mon Sep 17 00:00:00 2001 From: Michael Pankov Date: Sat, 15 Apr 2017 18:56:14 +0300 Subject: [PATCH 4/6] Add a closure destructor wrapper --- hlua/src/functions_write.rs | 16 +++++++++++++--- hlua/src/userdata.rs | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hlua/src/functions_write.rs b/hlua/src/functions_write.rs index c7e6af9..561edd6 100644 --- a/hlua/src/functions_write.rs +++ b/hlua/src/functions_write.rs @@ -170,6 +170,16 @@ pub trait FunctionExt

{ fn call_mut(&mut self, params: P) -> Self::Output; } +// Called when an object inside Lua is being dropped. +#[inline] +extern "C" fn closure_destructor_wrapper(lua: *mut ffi::lua_State) -> libc::c_int { + unsafe { + let obj = ffi::lua_touserdata(lua, -1); + ptr::drop_in_place((obj as *mut u8) as *mut T); + 0 + } +} + macro_rules! impl_function_ext { () => ( impl FunctionExt<()> for Function where Z: FnMut() -> R { @@ -214,7 +224,7 @@ macro_rules! impl_function_ext { Err(_) => unreachable!(), }; - ffi::lua_pushcfunction(lua.as_mut_lua().0, ::userdata::destructor_wrapper::); + ffi::lua_pushcfunction(lua.as_mut_lua().0, closure_destructor_wrapper::); ffi::lua_settable(lua.as_mut_lua().0, -3); } ffi::lua_setmetatable(lua_raw.0, -2); @@ -281,7 +291,7 @@ macro_rules! impl_function_ext { Err(_) => unreachable!(), }; - ffi::lua_pushcfunction(lua.as_mut_lua().0, ::userdata::destructor_wrapper::); + ffi::lua_pushcfunction(lua.as_mut_lua().0, closure_destructor_wrapper::); ffi::lua_settable(lua.as_mut_lua().0, -3); } ffi::lua_setmetatable(lua_raw.0, -2); @@ -552,7 +562,7 @@ mod tests { } } { - let foo = Foo { }; + let foo = ::std::rc::Rc::new(Foo { }); { let mut lua = Lua::new(); diff --git a/hlua/src/userdata.rs b/hlua/src/userdata.rs index 0253899..799c889 100644 --- a/hlua/src/userdata.rs +++ b/hlua/src/userdata.rs @@ -18,7 +18,7 @@ use LuaTable; // Called when an object inside Lua is being dropped. #[inline] -pub extern "C" fn destructor_wrapper(lua: *mut ffi::lua_State) -> libc::c_int { +extern "C" fn destructor_wrapper(lua: *mut ffi::lua_State) -> libc::c_int { unsafe { let obj = ffi::lua_touserdata(lua, -1); ptr::drop_in_place(obj as *mut TypeId); From 21cd68bb82c21f2b304912b73395029715536d09 Mon Sep 17 00:00:00 2001 From: Michael Pankov Date: Sat, 15 Apr 2017 18:58:01 +0300 Subject: [PATCH 5/6] Use Arc in test --- hlua/src/functions_write.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hlua/src/functions_write.rs b/hlua/src/functions_write.rs index 561edd6..38e753f 100644 --- a/hlua/src/functions_write.rs +++ b/hlua/src/functions_write.rs @@ -562,7 +562,7 @@ mod tests { } } { - let foo = ::std::rc::Rc::new(Foo { }); + let foo = ::std::sync::Arc::new(Foo { }); { let mut lua = Lua::new(); From a5952bf7c018ba955e12a2e94af149b8741c528c Mon Sep 17 00:00:00 2001 From: Michael Pankov Date: Sat, 15 Apr 2017 19:10:32 +0300 Subject: [PATCH 6/6] Use Arc --- hlua/src/functions_write.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hlua/src/functions_write.rs b/hlua/src/functions_write.rs index 38e753f..47bf84d 100644 --- a/hlua/src/functions_write.rs +++ b/hlua/src/functions_write.rs @@ -428,6 +428,8 @@ mod tests { use function1; use function2; + use std::sync::Arc; + #[test] fn simple_function() { let mut lua = Lua::new(); @@ -562,7 +564,7 @@ mod tests { } } { - let foo = ::std::sync::Arc::new(Foo { }); + let foo = Arc::new(Foo { }); { let mut lua = Lua::new();