Foolproofed config.allocator when it is a function

4 files changed, 37 insertions, 20 deletions

diff --git a/src/lanes.lua b/src/lanes.lua
index 6342ca9..7c1f8df 100644
--- a/src/lanes.lua
+++ b/src/lanes.lua
@@ -108,7 +108,7 @@ local default_params =
 -- #################################################################################################
 
 local boolean_param_checker = function(val_)
-    -- non-'boolean-false' should be 'boolean-true' or nil
+    -- non-'boolean-false|nil' should be 'boolean-true'
     return (not val_) or (val_ == true)
 end
 
@@ -116,7 +116,7 @@ local param_checkers =
 {
     allocator = function(val_)
         -- can be nil, "protected", or a function
-        return val_ and (type(val_) == "function" or val_ == "protected") or true
+        return (val_ == nil) or (type(val_) == "function" or val_ == "protected")
     end,
     internal_allocator = function(val_)
         -- can be "libc" or "allocator"
@@ -128,11 +128,11 @@ local param_checkers =
     end,
     nb_user_keepers = function(val_)
         -- nb_user_keepers should be a number in [0,100] (so that nobody tries to run OOM by specifying a huge amount)
-        return type(val_) == "number" and val_ >= 0 and val_ <= 100
+        return (type(val_) == "number") and (val_ >= 0) and (val_ <= 100)
     end,
     on_state_create = function(val_)
         -- on_state_create may be nil or a function
-        return val_ and type(val_) == "function" or true
+        return (val_ == nil) or (type(val_) == "function")
     end,
     shutdown_mode = function(val_)
         local valid_hooks = { soft = true, hard = true, call = true, ret = true, line = true, count = true }
diff --git a/src/state.cpp b/src/state.cpp
index 3f6b3d7..ee0b199 100644
--- a/src/state.cpp
+++ b/src/state.cpp
@@ -208,6 +208,9 @@ namespace state {
                             lua_pushcclosure(from, U->provideAllocator, 0);
                             lua_call(from, 0, 1);
                             AllocatorDefinition* const _def{ luaG_tofulluserdata<AllocatorDefinition>(from, -1) };
+                            if (!_def || _def->version != AllocatorDefinition::kAllocatorVersion) {
+                                raise_luaL_error(from, "Bad config.allocator function, must provide a valid AllocatorDefinition");
+                            }
                             lua_State* const _L{ lua_newstate(_def->allocF, _def->allocUD) };
                             lua_pop(from, 1);
                             return _L;
diff --git a/src/universe.cpp b/src/universe.cpp
index c98e2c8..d24a784 100644
--- a/src/universe.cpp
+++ b/src/universe.cpp
@@ -176,9 +176,23 @@ Universe::Universe()
 // Do I need to disable this when compiling for LuaJIT to prevent issues?
 void Universe::initializeAllocatorFunction(lua_State* const L_)
 {
+    // start by just grabbing whatever allocator was provided to lua_newstate
+    protectedAllocator.initFrom(L_);
     STACK_CHECK_START_REL(L_, 1);                                                                  // L_: settings
-    if (luaG_getfield(L_, -1, "allocator") != LuaType::NIL) {                                      // L_: settings allocator|nil|"protected"
-        // store C function pointer in an internal variable
+    switch (luaG_getfield(L_, -1, "allocator")) {                                                  // L_: settings allocator|nil|"protected"
+    case LuaType::NIL:
+        // nothing else to do
+        break;
+
+    case LuaType::STRING:
+        LUA_ASSERT(L_, luaG_tostring(L_, -1) == "protected");
+        // set the original allocator to call from inside protection by the mutex
+        protectedAllocator.installIn(L_);
+        // before a state is created, this function will be called to obtain the allocator
+        provideAllocator = luaG_provide_protected_allocator;
+        break;
+
+    case LuaType::FUNCTION:
         provideAllocator = lua_tocfunction(L_, -1);                                                // L_: settings allocator
         if (provideAllocator != nullptr) {
             // make sure the function doesn't have upvalues
@@ -190,17 +204,13 @@ void Universe::initializeAllocatorFunction(lua_State* const L_)
             // when we transfer the config table in newly created Lua states
             lua_pushnil(L_);                                                                       // L_: settings allocator nil
             lua_setfield(L_, -3, "allocator");                                                     // L_: settings allocator
-        } else if (luaG_type(L_, -1) == LuaType::STRING) { // should be "protected"
-            LUA_ASSERT(L_, strcmp(lua_tostring(L_, -1), "protected") == 0);
-            // set the original allocator to call from inside protection by the mutex
-            protectedAllocator.initFrom(L_);
-            protectedAllocator.installIn(L_);
-            // before a state is created, this function will be called to obtain the allocator
-            provideAllocator = luaG_provide_protected_allocator;
+        } else {
+            raise_luaL_error(L_, "Bad config.allocator, must be a C function");
         }
-    } else {
-        // just grab whatever allocator was provided to lua_newstate
-        protectedAllocator.initFrom(L_);
+        break;
+
+    default: // should be filtered out in lanes.lua
+        raise_luaL_error(L_, "Bad config.allocator type %s", luaG_typename(L_, -1).data());
     }
     lua_pop(L_, 1); // L_: settings
     STACK_CHECK(L_, 1);
@@ -208,7 +218,7 @@ void Universe::initializeAllocatorFunction(lua_State* const L_)
     std::ignore = luaG_getfield(L_, -1, "internal_allocator");                                     // L_: settings "libc"|"allocator"
     std::string_view const _allocator{ luaG_tostring(L_, -1) };
     if (_allocator == "libc") {
-        internalAllocator = AllocatorDefinition{ libc_lua_Alloc, nullptr };
+        internalAllocator = AllocatorDefinition{ AllocatorDefinition::kAllocatorVersion, libc_lua_Alloc, nullptr };
     } else if (provideAllocator == luaG_provide_protected_allocator) {
         // user wants mutex protection on the state's allocator. Use protection for our own allocations too, just in case.
         internalAllocator = protectedAllocator.makeDefinition();
diff --git a/src/universe.h b/src/universe.h
index 47b86af..3742ddd 100644
--- a/src/universe.h
+++ b/src/universe.h
@@ -18,6 +18,9 @@ class Lane;
 class AllocatorDefinition
 {
     public:
+    // xxh64 of string "kAllocatorVersion_1" generated at https://www.pelock.com/products/hash-calculator
+    static constexpr uintptr_t kAllocatorVersion{ static_cast<uintptr_t>(0xCF9D321B0DFB5715ull) };
+    uintptr_t version{ kAllocatorVersion };
     lua_Alloc allocF{ nullptr };
     void* allocUD{ nullptr };
 
@@ -27,8 +30,9 @@ class AllocatorDefinition
     // can't actually delete the operator because the compiler generates stack unwinding code that could call it in case of exception
     static void operator delete([[maybe_unused]] void* p_, [[maybe_unused]] lua_State* L_) { LUA_ASSERT(L_, !"should never be called"); }
 
-    AllocatorDefinition(lua_Alloc allocF_, void* allocUD_) noexcept
-    : allocF{ allocF_ }
+    AllocatorDefinition(uintptr_t const version_, lua_Alloc const allocF_, void* const allocUD_) noexcept
+    : version{ version_ }
+    , allocF{ allocF_ }
     , allocUD{ allocUD_ }
     {
     }
@@ -77,7 +81,7 @@ class ProtectedAllocator
 
     AllocatorDefinition makeDefinition()
     {
-        return AllocatorDefinition{ protected_lua_Alloc, this };
+        return AllocatorDefinition{ version, protected_lua_Alloc, this };
     }
 
     void installIn(lua_State* L_)