From 7acc7867a8ebee0a3467a382b6998cb4e95580ed Mon Sep 17 00:00:00 2001 From: Benoit Germain Date: Mon, 17 Mar 2025 14:54:56 +0100 Subject: Fix test "allocator = " not failing against LuaJIT like it should --- src/state.cpp | 2 +- src/universe.cpp | 10 +- src/universe.hpp | 2 +- unit_tests/init_and_shutdown.cpp | 347 ++++++++++++++++++++------------------- 4 files changed, 185 insertions(+), 176 deletions(-) diff --git a/src/state.cpp b/src/state.cpp index b558d11..fc7f5ef 100644 --- a/src/state.cpp +++ b/src/state.cpp @@ -158,7 +158,7 @@ namespace state { // for some reason, LuaJIT 64 bits does not support creating a state with lua_newstate... return luaL_newstate(); } else { - lanes::AllocatorDefinition const _def{ U->resolveAllocator(from, hint) }; + lanes::AllocatorDefinition const _def{ U->resolveAndValidateAllocator(from, hint) }; return _def.newState(); } } diff --git a/src/universe.cpp b/src/universe.cpp index dd7bc4b..db00b72 100644 --- a/src/universe.cpp +++ b/src/universe.cpp @@ -244,7 +244,7 @@ void Universe::initializeAllocatorFunction(lua_State* const L_) provideAllocator = lua_tocfunction(L_, -1); // L_: settings allocator if (provideAllocator != nullptr) { // make sure the function doesn't have upvalues - char const* _upname = lua_getupvalue(L_, -1, 1); // L_: settings allocator upval? + char const* _upname{ lua_getupvalue(L_, -1, 1) }; // L_: settings allocator upval? if (_upname != nullptr) { // should be "" for C functions with upvalues if any raise_luaL_error(L_, "config.allocator() shouldn't have upvalues"); } @@ -266,11 +266,11 @@ void Universe::initializeAllocatorFunction(lua_State* const L_) std::ignore = luaG_getfield(L_, kIdxTop, "internal_allocator"); // L_: settings "libc"|"allocator" LUA_ASSERT(L_, lua_isstring(L_, kIdxTop)); // should be the case due to lanes.lua parameter validation std::string_view const _allocator{ luaG_tostring(L_, kIdxTop) }; + // use whatever the provider provides. This performs validation of what provideAllocator is giving + // we do this even if _allocator == "libc", to have the validation part + internalAllocator = resolveAndValidateAllocator(L_, "internal"); if (_allocator == "libc") { internalAllocator = lanes::AllocatorDefinition{ libc_lua_Alloc, nullptr }; - } else { - // use whatever the provider provides - internalAllocator = resolveAllocator(L_, "internal"); } lua_pop(L_, 1); // L_: settings STACK_CHECK(L_, 1); @@ -331,7 +331,7 @@ void Universe::initializeOnStateCreate(lua_State* const L_) // ################################################################################################# -lanes::AllocatorDefinition Universe::resolveAllocator(lua_State* const L_, std::string_view const& hint_) const +lanes::AllocatorDefinition Universe::resolveAndValidateAllocator(lua_State* const L_, std::string_view const& hint_) const { lanes::AllocatorDefinition _ret{ protectedAllocator }; if (provideAllocator == nullptr) { diff --git a/src/universe.hpp b/src/universe.hpp index 2a3085d..42a3d83 100644 --- a/src/universe.hpp +++ b/src/universe.hpp @@ -151,7 +151,7 @@ class Universe final static int InitializeFinalizer(lua_State* L_); void initializeOnStateCreate(lua_State* L_); [[nodiscard]] - lanes::AllocatorDefinition resolveAllocator(lua_State* L_, std::string_view const& hint_) const; + lanes::AllocatorDefinition resolveAndValidateAllocator(lua_State* L_, std::string_view const& hint_) const; static inline void Store(lua_State* L_, Universe* U_); [[nodiscard]] bool terminateFreeRunningLanes(lua_Duration shutdownTimeout_, CancelOp op_); diff --git a/unit_tests/init_and_shutdown.cpp b/unit_tests/init_and_shutdown.cpp index 9652f2d..b8174fd 100644 --- a/unit_tests/init_and_shutdown.cpp +++ b/unit_tests/init_and_shutdown.cpp @@ -56,234 +56,243 @@ TEST_CASE("lanes.require 'lanes'") } // ################################################################################################# -// ################################################################################################# -// allocator should be "protected", a C function returning a suitable userdata, or nil -TEST_CASE("lanes.configure") +// allocator should be "protected", a C function returning a suitable userdata, or nil +TEST_CASE("lanes.configure.allocator") { LuaState L{ LuaState::WithBaseLibs{ true }, LuaState::WithFixture{ false } }; + SECTION("allocator = false") + { + L.requireFailure("require 'lanes'.configure{allocator = false}"); + } + // --------------------------------------------------------------------------------------------- - SECTION("allocator", "[allocator]") + SECTION("allocator = true") { - SECTION("allocator = false") - { - L.requireFailure("require 'lanes'.configure{allocator = false}"); - } + L.requireFailure("require 'lanes'.configure{allocator = true}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator = true") - { - L.requireFailure("require 'lanes'.configure{allocator = true}"); - } + SECTION("allocator = ") + { + L.requireFailure("require 'lanes'.configure{allocator = 33}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator = ") - { - L.requireFailure("require 'lanes'.configure{allocator = 33}"); - } + SECTION("allocator = ") + { + L.requireFailure("require 'lanes'.configure{allocator = {}}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator =
") - { - L.requireFailure("require 'lanes'.configure{allocator = {}}"); - } + SECTION("allocator = ") + { + L.requireFailure("require 'lanes'.configure{allocator = function() return {}, 12, 'yoy' end}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator = ") - { - L.requireFailure("require 'lanes'.configure{allocator = function() return {}, 12, 'yoy' end}"); - } + SECTION("allocator = ") + { + // a C function that doesn't return what we expect should cause an error too + // TODO: for some reason, we use os.getenv here because using 'print' as the culprit, the tests deadlock in Release builds + L.requireFailure("return type(require 'lanes'.configure{allocator = os.getenv})"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator = ") - { - // a C function that doesn't return what we expect should cause an error too - L.requireFailure("require 'lanes'.configure{allocator = print}"); - } + SECTION("allocator = ") + { + // oops, a typo + L.requireFailure("require 'lanes'.configure{allocator = 'Protected'}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator = ") - { - // oops, a typo - L.requireFailure("require 'lanes'.configure{allocator = 'Protected'}"); - } + SECTION("allocator = 'protected'") + { + // no typo, should work + L.requireSuccess("require 'lanes'.configure{allocator = 'protected'}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator = 'protected'") - { - // no typo, should work - L.requireSuccess("require 'lanes'.configure{allocator = 'protected'}"); - } + SECTION("allocator = ") + { + // a function that provides what we expect is fine + static constexpr lua_CFunction _provideAllocator = +[](lua_State* const L_) { + lanes::AllocatorDefinition* const _def{ new (L_) lanes::AllocatorDefinition{} }; + _def->initFrom(L_); + return 1; + }; + lua_pushcfunction(L, _provideAllocator); + lua_setglobal(L, "ProvideAllocator"); + L.requireSuccess("require 'lanes'.configure{allocator = ProvideAllocator}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator = ") - { - // a function that provides what we expect is fine - static constexpr lua_CFunction _provideAllocator = +[](lua_State* const L_) { - lanes::AllocatorDefinition* const _def{ new (L_) lanes::AllocatorDefinition{} }; - _def->initFrom(L_); - return 1; - }; - lua_pushcfunction(L, _provideAllocator); - lua_setglobal(L, "ProvideAllocator"); - L.requireSuccess("require 'lanes'.configure{allocator = ProvideAllocator}"); - } + SECTION("allocator not returning an AllocatorDefinition") + { + // a function that provides something that is definitely not an AllocatorDefinition, should cause an error + static constexpr lua_CFunction _provideAllocator = +[](lua_State* const L_) { + lua_newtable(L_); + return 1; + }; + lua_pushcfunction(L, _provideAllocator); + lua_setglobal(L, "ProvideAllocator"); + // force value of internal_allocator so that the LuaJIT-default 'libc' is not selected + // which would prevent us from calling _provideAllocator + L.requireFailure("require 'lanes'.configure{allocator = ProvideAllocator, internal_allocator = 'allocator'}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("allocator not returning an AllocatorDefinition") - { - // a function that provides something that is definitely not an AllocatorDefinition, should cause an error - static constexpr lua_CFunction _provideAllocator = +[](lua_State* const L_) { - lua_newtable(L_); - return 1; - }; - lua_pushcfunction(L, _provideAllocator); - lua_setglobal(L, "ProvideAllocator"); - // force value of internal_allocator so that the LuaJIT-default 'libc' is not selected - // which would prevent us from calling _provideAllocator - L.requireFailure("require 'lanes'.configure{allocator = ProvideAllocator, internal_allocator = 'allocator'}"); - } + SECTION("allocator returning an AllocatorDefinition with the wrong signature") + { + // a function that provides something that is too small to contain an AllocatorDefinition, should cause an error + static constexpr lua_CFunction _provideAllocator = +[](lua_State* const L_) { + // create a full userdata that is too small (it only contains enough to store a version tag, but not the rest + auto* const _duck{ static_cast(lua_newuserdata(L_, sizeof(lanes::AllocatorDefinition::version_t))) }; + *_duck = 666777; + return 1; + }; + lua_pushcfunction(L, _provideAllocator); + lua_setglobal(L, "ProvideAllocator"); + // force value of internal_allocator so that the LuaJIT-default 'libc' is not selected + // which would prevent us from calling _provideAllocator + L.requireFailure("require 'lanes'.configure{allocator = ProvideAllocator, internal_allocator = 'allocator'}"); + } - // ----------------------------------------------------------------------------------------- + // ----------------------------------------------------------------------------------------- - SECTION("allocator returning an AllocatorDefinition with the wrong signature") - { - // a function that provides something that is too small to contain an AllocatorDefinition, should cause an error - static constexpr lua_CFunction _provideAllocator = +[](lua_State* const L_) { - // create a full userdata that is too small (it only contains enough to store a version tag, but not the rest - auto* const _duck{ static_cast(lua_newuserdata(L_, sizeof(lanes::AllocatorDefinition::version_t))) }; - *_duck = 666777; - return 1; - }; - lua_pushcfunction(L, _provideAllocator); - lua_setglobal(L, "ProvideAllocator"); - // force value of internal_allocator so that the LuaJIT-default 'libc' is not selected - // which would prevent us from calling _provideAllocator - L.requireFailure("require 'lanes'.configure{allocator = ProvideAllocator, internal_allocator = 'allocator'}"); - } + SECTION("allocator returning something too small to be a valid AllocatorDefinition") + { + // a function that provides something that attempts to pass as an AllocatorDefinition, but is not, should cause an error + static constexpr lua_CFunction _provideAllocator = +[](lua_State* const L_) { + // create a full userdata of the correct size, but of course the contents don't match + int* const _duck{ static_cast(lua_newuserdata(L_, sizeof(lanes::AllocatorDefinition))) }; + _duck[0] = 666; + _duck[1] = 777; + return 1; + }; + lua_pushcfunction(L, _provideAllocator); + lua_setglobal(L, "ProvideAllocator"); + // force value of internal_allocator so that the LuaJIT-default 'libc' is not selected + // which would prevent us from calling _provideAllocator + L.requireFailure("require 'lanes'.configure{allocator = ProvideAllocator, internal_allocator = 'allocator'}"); + } +} - // ----------------------------------------------------------------------------------------- +// ################################################################################################# - SECTION("allocator returning something too small to be a valid AllocatorDefinition") - { - // a function that provides something that attempts to pass as an AllocatorDefinition, but is not, should cause an error - static constexpr lua_CFunction _provideAllocator = +[](lua_State* const L_) { - // create a full userdata of the correct size, but of course the contents don't match - int* const _duck{ static_cast(lua_newuserdata(L_, sizeof(lanes::AllocatorDefinition))) }; - _duck[0] = 666; - _duck[1] = 777; - return 1; - }; - lua_pushcfunction(L, _provideAllocator); - lua_setglobal(L, "ProvideAllocator"); - // force value of internal_allocator so that the LuaJIT-default 'libc' is not selected - // which would prevent us from calling _provideAllocator - L.requireFailure("require 'lanes'.configure{allocator = ProvideAllocator, internal_allocator = 'allocator'}"); - } - } +TEST_CASE("lanes.configure.internal_allocator") +{ + LuaState L{ LuaState::WithBaseLibs{ true }, LuaState::WithFixture{ false } }; - // --------------------------------------------------------------------------------------------- // internal_allocator should be a string, "libc"/"allocator" - SECTION("[internal_allocator") + SECTION("internal_allocator = false") { - SECTION("internal_allocator = false") - { - L.requireFailure("require 'lanes'.configure{internal_allocator = false}"); - } + L.requireFailure("require 'lanes'.configure{internal_allocator = false}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("internal_allocator = true") - { - L.requireFailure("require 'lanes'.configure{internal_allocator = true}"); - } + SECTION("internal_allocator = true") + { + L.requireFailure("require 'lanes'.configure{internal_allocator = true}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("internal_allocator =
") - { - L.requireFailure("require 'lanes'.configure{internal_allocator = {}}"); - } + SECTION("internal_allocator =
") + { + L.requireFailure("require 'lanes'.configure{internal_allocator = {}}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("internal_allocator = ") - { - L.requireFailure("require 'lanes'.configure{internal_allocator = function() end}"); - } + SECTION("internal_allocator = ") + { + L.requireFailure("require 'lanes'.configure{internal_allocator = function() end}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("internal_allocator = ") - { - L.requireFailure("require 'lanes'.configure{internal_allocator = 'gluh'}"); - } + SECTION("internal_allocator = ") + { + L.requireFailure("require 'lanes'.configure{internal_allocator = 'gluh'}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("internal_allocator = 'libc'") - { - L.requireSuccess("require 'lanes'.configure{internal_allocator = 'libc'}"); - } + SECTION("internal_allocator = 'libc'") + { + L.requireSuccess("require 'lanes'.configure{internal_allocator = 'libc'}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("internal_allocator = 'allocator'") - { - L.requireSuccess("require 'lanes'.configure{internal_allocator = 'allocator'}"); - } + SECTION("internal_allocator = 'allocator'") + { + L.requireSuccess("require 'lanes'.configure{internal_allocator = 'allocator'}"); } +} + +// ################################################################################################# + +TEST_CASE("lanes.configure.keepers_gc_threshold") +{ + LuaState L{ LuaState::WithBaseLibs{ true }, LuaState::WithFixture{ false } }; - // --------------------------------------------------------------------------------------------- // keepers_gc_threshold should be a number in [0, 100] - SECTION("keepers_gc_threshold") + SECTION("keepers_gc_threshold =
") { - SECTION("keepers_gc_threshold =
") - { - L.requireFailure("require 'lanes'.configure{keepers_gc_threshold = {}}"); - } + L.requireFailure("require 'lanes'.configure{keepers_gc_threshold = {}}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("keepers_gc_threshold = ") - { - L.requireFailure("require 'lanes'.configure{keepers_gc_threshold = 'gluh'}"); - } + SECTION("keepers_gc_threshold = ") + { + L.requireFailure("require 'lanes'.configure{keepers_gc_threshold = 'gluh'}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("keepers_gc_threshold = -1") - { - L.requireSuccess("require 'lanes'.configure{keepers_gc_threshold = -1}"); - } + SECTION("keepers_gc_threshold = -1") + { + L.requireSuccess("require 'lanes'.configure{keepers_gc_threshold = -1}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("keepers_gc_threshold = 0") - { - L.requireSuccess("require 'lanes'.configure{keepers_gc_threshold = 0}"); - } + SECTION("keepers_gc_threshold = 0") + { + L.requireSuccess("require 'lanes'.configure{keepers_gc_threshold = 0}"); + } - // ----------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------- - SECTION("keepers_gc_threshold = 100") - { - L.requireSuccess("require 'lanes'.configure{keepers_gc_threshold = 100}"); - } + SECTION("keepers_gc_threshold = 100") + { + L.requireSuccess("require 'lanes'.configure{keepers_gc_threshold = 100}"); } +} + +// ################################################################################################# + +TEST_CASE("lanes.configure.the rest") +{ + LuaState L{ LuaState::WithBaseLibs{ true }, LuaState::WithFixture{ false } }; + // --------------------------------------------------------------------------------------------- // nb_user_keepers should be a number in [0, 100] -- cgit v1.2.3-55-g6feb