From 1b059b5a62396bd26a697af3ad177e5a8bd8be51 Mon Sep 17 00:00:00 2001 From: Jeremy Ong Date: Sun, 26 Jan 2014 16:19:28 -0800 Subject: [PATCH] Fix a number of instances of memory leakage and add test coverage. --- include/Selector.h | 4 +++- include/State.h | 9 ++++---- include/util.h | 51 +++++++++++++++++++++++++++++++++++++++++++ src/Selector.cpp | 23 ++++++++++++++----- test/Test.cpp | 17 ++++++++++++--- test/class_tests.h | 9 +++----- test/interop_tests.h | 45 +++++++++++++------------------------- test/selector_tests.h | 42 ++++++++++++----------------------- 8 files changed, 122 insertions(+), 78 deletions(-) diff --git a/include/Selector.h b/include/Selector.h index b291cdf..0c0f5d7 100644 --- a/include/Selector.h +++ b/include/Selector.h @@ -63,7 +63,7 @@ class Selector { _get(); detail::_push_n(_state._l, std::forward(args)...); lua_call(_state._l, sizeof...(Args), sizeof...(Ret)); - return detail::_pop_n(_state._l); + return detail::_pop_n_reset(_state._l); } template @@ -73,6 +73,7 @@ class Selector { detail::_push(_state._l, t); }; _put(push); + lua_settop(_state._l, 0); } template @@ -83,6 +84,7 @@ class Selector { _state.Register(t, fun_tuple); }; _put(push); + lua_settop(_state._l, 0); } template diff --git a/include/State.h b/include/State.h index 9882bc6..31ff8b1 100644 --- a/include/State.h +++ b/include/State.h @@ -37,6 +37,10 @@ class State { State(State &&other); ~State(); + int Size() const { + return lua_gettop(_l); + } + bool Load(const std::string &file); void Push() {} // Base case @@ -61,11 +65,6 @@ class State { return result; } public: - template - typename detail::_pop_n_impl::type Pop() { - return detail::_pop_n(_l); - } - template void Register(std::function fun) { constexpr int arity = detail::_arity::value; diff --git a/include/util.h b/include/util.h index 124ecf0..52eeb04 100644 --- a/include/util.h +++ b/include/util.h @@ -51,6 +51,7 @@ struct _indices_builder<0, Is...> { template struct _id {}; + inline bool _get(_id, lua_State *l, const int index) { return lua_toboolean(l, index); } @@ -155,5 +156,55 @@ typename _pop_n_impl::type _pop_n(lua_State *l) { return _pop_n_impl::apply(l); } +template +struct _pop_n_reset_impl { + using type = std::tuple; + + template + static type worker(lua_State *l, + _indices) { + return std::make_tuple(_get(_id{}, l, N + 1)...); + } + + static type apply(lua_State *l) { + auto ret = worker(l, typename _indices_builder::type()); + lua_settop(l, 0); + return ret; + } +}; + +// Popping nothing returns void +template +struct _pop_n_reset_impl<0, Ts...> { + using type = void; + static type apply(lua_State *l) { + lua_settop(l, 0); + } +}; + +// Popping one element returns an unboxed value +template +struct _pop_n_reset_impl<1, T> { + using type = T; + static type apply(lua_State *l) { + T ret = _get(_id{}, l, -1); + lua_settop(l, 0); + return ret; + } +}; + +template +typename _pop_n_reset_impl::type +_pop_n_reset(lua_State *l) { + return _pop_n_reset_impl::apply(l); +} + +template +T _pop(_id t, lua_State *l) { + T ret = _get(t, l, -1); + lua_pop(l, 1); + return ret; +} + } } diff --git a/src/Selector.cpp b/src/Selector.cpp index fa8ffb7..df4e2d0 100644 --- a/src/Selector.cpp +++ b/src/Selector.cpp @@ -10,6 +10,8 @@ void Selector::_check_create_table() { lua_newtable(_state._l); }; _put(put); + } else { + lua_pop(_state._l, 1); } } @@ -31,6 +33,7 @@ void Selector::operator=(const char *s) const { detail::_push(_state._l, std::string{s}); }; _put(push); + lua_settop(_state._l, 0); } Selector::operator bool() const { @@ -40,7 +43,9 @@ Selector::operator bool() const { (*_functor)(1); _functor.release(); } - return detail::_get(detail::_id{}, _state._l, -1); + auto ret = detail::_pop(detail::_id{}, _state._l); + lua_settop(_state._l, 0); + return ret; } Selector::operator int() const { @@ -50,7 +55,9 @@ Selector::operator int() const { (*_functor)(1); _functor.release(); } - return detail::_get(detail::_id{}, _state._l, -1); + auto ret = detail::_pop(detail::_id{}, _state._l); + lua_settop(_state._l, 0); + return ret; } Selector::operator unsigned int() const { @@ -60,7 +67,9 @@ Selector::operator unsigned int() const { (*_functor)(1); _functor.release(); } - return detail::_get(detail::_id{}, _state._l, -1); + auto ret = detail::_pop(detail::_id{}, _state._l); + lua_settop(_state._l, 0); + return ret; } Selector::operator lua_Number() const { @@ -70,7 +79,9 @@ Selector::operator lua_Number() const { (*_functor)(1); _functor.release(); } - return detail::_get(detail::_id{}, _state._l, -1); + auto ret = detail::_pop(detail::_id{}, _state._l); + lua_settop(_state._l, 0); + return ret; } Selector::operator std::string() const { @@ -80,7 +91,9 @@ Selector::operator std::string() const { (*_functor)(1); _functor.release(); } - return detail::_get(detail::_id{}, _state._l, -1); + auto ret = detail::_pop(detail::_id{}, _state._l); + lua_settop(_state._l, 0); + return ret; } Selector Selector::operator[](const char *name) { diff --git a/test/Test.cpp b/test/Test.cpp index 5dc8ab9..c103030 100644 --- a/test/Test.cpp +++ b/test/Test.cpp @@ -5,7 +5,9 @@ #include // The most ghetto unit testing framework ever! -using Test = bool(*)(); +// To add a test, author a function with the Test function signature +// and include it in the Map of tests below. +using Test = bool(*)(sel::State &); using TestMap = std::map; static TestMap tests = { {"test_function_no_args", test_function_no_args}, @@ -48,22 +50,31 @@ void ExecuteAll() { const int num_tests = tests.size(); int passing = 0; for (auto it = tests.begin(); it != tests.end(); ++it) { - const bool result = it->second(); + sel::State state{true}; + const bool result = it->second(state); if (result) passing += 1; else std::cout << "Test \"" << it->first << "\" failed." << std::endl; + int size = state.Size(); + if (size != 0) + std::cout << "Test \"" << it->first + << "\" leaked " << size << " values" << std::endl; } std::cout << passing << " out of " << num_tests << " tests finished successfully." << std::endl; } bool ExecuteTest(const char *test) { + sel::State state{true}; auto it = tests.find(test); - return it->second(); + return it->second(state); } int main() { + // Executing all tests will run all test cases and check leftover + // stack size afterwards. It is expected that the stack size + // post-test is 0. ExecuteAll(); } diff --git a/test/class_tests.h b/test/class_tests.h index 97adc06..5d3006c 100644 --- a/test/class_tests.h +++ b/test/class_tests.h @@ -13,25 +13,22 @@ struct Foo { } }; -bool test_register_class() { +bool test_register_class(sel::State &state) { Foo foo_instance(1); - sel::State state; state["foo_instance"].SetObj(foo_instance, "double_add", &Foo::DoubleAdd); const int answer = state["foo_instance"]["double_add"](3); return (answer == 8); } -bool test_mutate_instance() { +bool test_mutate_instance(sel::State &state) { Foo foo_instance(1); - sel::State state; state["foo_instance"].SetObj(foo_instance, "set_x", &Foo::SetX); state["foo_instance"]["set_x"].Call(4); return (foo_instance.x == 4); } -bool test_multiple_methods() { +bool test_multiple_methods(sel::State &state) { Foo foo_instance(1); - sel::State state; state["foo_instance"].SetObj(foo_instance, "double_add", &Foo::DoubleAdd, "set_x", &Foo::SetX); diff --git a/test/interop_tests.h b/test/interop_tests.h index baa5ec0..f26f4ec 100644 --- a/test/interop_tests.h +++ b/test/interop_tests.h @@ -10,29 +10,25 @@ int my_add(int a, int b) { void no_return() { } -bool test_function_no_args() { - sel::State state; +bool test_function_no_args(sel::State &state) { state.Load("../test/test.lua"); state["foo"](); return true; } -bool test_add() { - sel::State state; +bool test_add(sel::State &state) { state.Load("../test/test.lua"); return state["add"](5, 2) == 7; } -bool test_multi_return() { - sel::State state; +bool test_multi_return(sel::State &state) { state.Load("../test/test.lua"); int sum, difference; std::tie(sum, difference) = state["sum_and_difference"].Call(3, 1); return (sum == 4 && difference == 2); } -bool test_heterogeneous_return() { - sel::State state; +bool test_heterogeneous_return(sel::State &state) { state.Load("../test/test.lua"); int x; bool y; @@ -41,38 +37,33 @@ bool test_heterogeneous_return() { return x == 4 && y == true && z == "hi"; } -bool test_call_field() { - sel::State state; +bool test_call_field(sel::State &state) { state.Load("../test/test.lua"); int answer = state["mytable"]["foo"](); return answer == 4; } -bool test_call_c_function() { - sel::State state; +bool test_call_c_function(sel::State &state) { state.Load("../test/test.lua"); state["cadd"] = std::function(my_add); int answer = state["cadd"](3, 6); return answer == 9; } -bool test_call_c_fun_from_lua() { - sel::State state; +bool test_call_c_fun_from_lua(sel::State &state) { state.Load("../test/test.lua"); state["cadd"] = std::function(my_add); int answer = state["execute"](); return answer == 11; } -bool test_no_return() { - sel::State state; +bool test_no_return(sel::State &state) { state["no_return"] = &no_return; state["no_return"].Call(); return true; } -bool test_call_lambda() { - sel::State state; +bool test_call_lambda(sel::State &state) { state.Load("../test/test.lua"); std::function mult = [](int x, int y){ return x * y; }; state["cmultiply"] = mult; @@ -80,18 +71,16 @@ bool test_call_lambda() { return answer == 30; } -bool test_call_normal_c_fun() { - sel::State state; +bool test_call_normal_c_fun(sel::State &state) { state.Load("../test/test.lua"); state["cadd"] = &my_add; const int answer = state["cadd"](4, 20); return answer == 24; } -bool test_call_normal_c_fun_many_times() { +bool test_call_normal_c_fun_many_times(sel::State &state) { // Ensures there isn't any strange overflow problem or lingering // state - sel::State state; state.Load("../test/test.lua"); state["cadd"] = &my_add; bool result = true; @@ -102,7 +91,7 @@ bool test_call_normal_c_fun_many_times() { return result; } -bool test_call_functor() { +bool test_call_functor(sel::State &state) { struct the_answer { int answer = 42; int operator()() { @@ -110,7 +99,6 @@ bool test_call_functor() { } }; the_answer functor; - sel::State state; state.Load("../test/test.lua"); state["c_the_answer"] = std::function(functor); int answer = state["c_the_answer"](); @@ -122,8 +110,7 @@ std::tuple my_sum_and_difference(int x, int y) { return std::make_tuple(x+y, x-y); } -bool test_multivalue_c_fun_return() { - sel::State state; +bool test_multivalue_c_fun_return(sel::State &state) { state.Load("../test/test.lua"); state["test_fun"] = &my_sum_and_difference; int sum, difference; @@ -131,16 +118,14 @@ bool test_multivalue_c_fun_return() { return sum == 0 && difference == -4; } -bool test_multivalue_c_fun_from_lua() { - sel::State state; +bool test_multivalue_c_fun_from_lua(sel::State &state) { state.Load("../test/test.lua"); state["doozy_c"] = &my_sum_and_difference; int answer = state["doozy"](5); return answer == -75; } -bool test_embedded_nulls() { - sel::State state; +bool test_embedded_nulls(sel::State &state) { state.Load("../test/test.lua"); std::string result = state["embedded_nulls"](); return result.size() == 4; diff --git a/test/selector_tests.h b/test/selector_tests.h index 557cbfe..c5607e7 100644 --- a/test/selector_tests.h +++ b/test/selector_tests.h @@ -2,97 +2,83 @@ #include -bool test_select_global() { - sel::State state; +bool test_select_global(sel::State &state) { state.Load("../test/test.lua"); int answer = state["my_global"]; return answer == 4; } -bool test_select_field() { - sel::State state; +bool test_select_field(sel::State &state) { state.Load("../test/test.lua"); lua_Number answer = state["my_table"]["key"]; return answer == lua_Number(6.4); } -bool test_select_index() { - sel::State state; +bool test_select_index(sel::State &state) { state.Load("../test/test.lua"); std::string answer = state["my_table"][3]; return answer == "hi"; } -bool test_select_nested_field() { - sel::State state; +bool test_select_nested_field(sel::State &state) { state.Load("../test/test.lua"); std::string answer = state["my_table"]["nested"]["foo"]; return answer == "bar"; } -bool test_select_nested_index() { - sel::State state; +bool test_select_nested_index(sel::State &state) { state.Load("../test/test.lua"); int answer = state["my_table"]["nested"][2]; return answer == -3; } -bool test_select_equality() { - sel::State state; +bool test_select_equality(sel::State &state) { state.Load("../test/test.lua"); return state["my_table"]["nested"][2] == -3; } -bool test_select_cast() { - sel::State state; +bool test_select_cast(sel::State &state) { state.Load("../test/test.lua"); return int(state["global1"]) == state["global2"]; } -bool test_set_global() { - sel::State state; +bool test_set_global(sel::State &state) { state.Load("../test/test.lua"); auto lua_dummy_global = state["dummy_global"]; lua_dummy_global = 32; return state["dummy_global"] == 32; } -bool test_set_field() { - sel::State state; +bool test_set_field(sel::State &state) { state.Load("../test/test.lua"); state["my_table"]["dummy_key"] = "testing"; return state["my_table"]["dummy_key"] == "testing"; } -bool test_set_index() { - sel::State state; +bool test_set_index(sel::State &state) { state.Load("../test/test.lua"); state["my_table"][10] = 3; return state["my_table"][10] == 3; } -bool test_set_nested_field() { - sel::State state; +bool test_set_nested_field(sel::State &state) { state.Load("../test/test.lua"); state["my_table"]["nested"]["asdf"] = true; return state["my_table"]["nested"]["asdf"]; } -bool test_set_nested_index() { - sel::State state; +bool test_set_nested_index(sel::State &state) { state.Load("../test/test.lua"); state["my_table"]["nested"][1] = 2; return state["my_table"]["nested"][1] == 2; } -bool test_create_table_field() { - sel::State state; +bool test_create_table_field(sel::State &state) { state["new_table"]["test"] = 4; return state["new_table"]["test"] == 4; } -bool test_create_table_index() { - sel::State state; +bool test_create_table_index(sel::State &state) { state["new_table"][3] = 4; return state["new_table"][3] == 4; }