C++ integration: cleanup in Linda/Keeper interaction

4 files changed, 164 insertions, 150 deletions

diff --git a/src/keeper.cpp b/src/keeper.cpp
index c5bbb9d..c306d58 100644
--- a/src/keeper.cpp
+++ b/src/keeper.cpp
@@ -40,6 +40,7 @@
 #include "keeper.h"
 
 #include "compat.h"
+#include "linda.h"
 #include "state.h"
 #include "tools.h"
 #include "uniquekey.h"
@@ -207,16 +208,17 @@ static void push_table(lua_State* L, int idx_)
 
 // #################################################################################################
 
-int keeper_push_linda_storage(Universe* U, DestState L, void* ptr_, uintptr_t magic_)
+// only used by linda:dump() and linda:__towatch() for debugging purposes
+int keeper_push_linda_storage(Linda& linda_, DestState L)
 {
-    Keeper* const K{ which_keeper(U->keepers, magic_) };
+    Keeper* const K{ linda_.whichKeeper() };
     SourceState const KL{ K ? K->L : nullptr };
     if (KL == nullptr)
         return 0;
     STACK_GROW(KL, 4);                                                      // KEEPER                       MAIN
     STACK_CHECK_START_REL(KL, 0);
     FIFOS_KEY.pushValue(KL);                                                // fifos
-    lua_pushlightuserdata(KL, ptr_);                                        // fifos ud
+    lua_pushlightuserdata(KL, &linda_);                                     // fifos ud
     lua_rawget(KL, -2);                                                     // fifos storage
     lua_remove(KL, -2);                                                     // storage
     if (!lua_istable(KL, -1))
@@ -229,7 +231,7 @@ int keeper_push_linda_storage(Universe* U, DestState L, void* ptr_, uintptr_t ma
     STACK_GROW(L, 5);
     STACK_CHECK_START_REL(L, 0);
     lua_newtable(L);                                                                                     // out
-    InterCopyContext c{ U, L, KL, {}, {}, {}, LookupMode::FromKeeper, {} };
+    InterCopyContext c{ linda_.U, L, KL, {}, {}, {}, LookupMode::FromKeeper, {} };
     lua_pushnil(KL);                                                        // storage nil
     while (lua_next(KL, -2))                                                // storage key fifo
     {
@@ -748,37 +750,14 @@ void init_keepers(Universe* U, lua_State* L)
 
 // #################################################################################################
 
-// should be called only when inside a keeper_acquire/keeper_release pair (see Linda::ProtectedCall)
-Keeper* which_keeper(Keepers* keepers_, uintptr_t magic_)
+Keeper* Linda::acquireKeeper() const
 {
-    int const nbKeepers{ keepers_->nb_keepers };
-    if (nbKeepers)
-    {
-        unsigned int i = (unsigned int) ((magic_ >> KEEPER_MAGIC_SHIFT) % nbKeepers);
-        return &keepers_->keeper_array[i];
-    }
-    return nullptr;
-}
-
-// #################################################################################################
-
-Keeper* keeper_acquire(Keepers* keepers_, uintptr_t magic_)
-{
-    int const nbKeepers{ keepers_->nb_keepers };
+    int const nbKeepers{ U->keepers->nb_keepers };
     // can be 0 if this happens during main state shutdown (lanes is being GC'ed -> no keepers)
     if (nbKeepers)
     {
-        /*
-        * Any hashing will do that maps pointers to 0..GNbKeepers-1 
-        * consistently.
-        *
-        * Pointers are often aligned by 8 or so - ignore the low order bits
-        * have to cast to unsigned long to avoid compilation warnings about loss of data when converting pointer-to-integer
-        */
-        unsigned int i = (unsigned int)((magic_ >> KEEPER_MAGIC_SHIFT) % nbKeepers);
-        Keeper* K = &keepers_->keeper_array[i];
+        Keeper* const K{ &U->keepers->keeper_array[m_keeper_index] };
         K->m_mutex.lock();
-        //++ K->count;
         return K;
     }
     return nullptr;
@@ -786,12 +765,12 @@ Keeper* keeper_acquire(Keepers* keepers_, uintptr_t magic_)
 
 // #################################################################################################
 
-void keeper_release(Keeper* K)
+void Linda::releaseKeeper(Keeper* K_) const
 {
-    //-- K->count;
-    if (K)
+    if (K_) // can be nullptr if we tried to acquire during shutdown
     {
-        K->m_mutex.unlock();
+        assert(K_ == &U->keepers->keeper_array[m_keeper_index]);
+        K_->m_mutex.unlock();
     }
 }
 
diff --git a/src/keeper.h b/src/keeper.h
index ef86f94..4c79fd7 100644
--- a/src/keeper.h
+++ b/src/keeper.h
@@ -15,6 +15,7 @@ extern "C" {
 #include <mutex>
 
 // forwards
+class Linda;
 enum class LookupMode;
 class Universe;
 
@@ -34,18 +35,14 @@ struct Keepers
     Keeper keeper_array[1];
 };
 
-static constexpr uintptr_t KEEPER_MAGIC_SHIFT{ 3 };
 // crc64/we of string "NIL_SENTINEL" generated at http://www.nitrxgen.net/hashgen/
 static constexpr UniqueKey NIL_SENTINEL{ 0x7EAAFA003A1D11A1ull, "linda.null" };
 
 void init_keepers(Universe* U, lua_State* L);
 void close_keepers(Universe* U);
 
-[[nodiscard]] Keeper* which_keeper(Keepers* keepers_, uintptr_t magic_);
-[[nodiscard]] Keeper* keeper_acquire(Keepers* keepers_, uintptr_t magic_);
-void keeper_release(Keeper* K_);
 void keeper_toggle_nil_sentinels(lua_State* L, int val_i_, LookupMode const mode_);
-[[nodiscard]] int keeper_push_linda_storage(Universe* U, DestState L, void* ptr_, uintptr_t magic_);
+[[nodiscard]] int keeper_push_linda_storage(Linda& linda_, DestState L);
 
 using keeper_api_t = lua_CFunction;
 #define KEEPER_API(_op) keepercall_##_op
diff --git a/src/linda.cpp b/src/linda.cpp
index a19b126..0a1ca3f 100644
--- a/src/linda.cpp
+++ b/src/linda.cpp
@@ -30,21 +30,22 @@ THE SOFTWARE.
 ===============================================================================
 */
 
+#include "linda.h"
+
 #include "compat.h"
-#include "deep.h"
 #include "keeper.h"
 #include "lanes_private.h"
 #include "threading.h"
 #include "tools.h"
 #include "universe.h"
 
-#include <array>
 #include <functional>
-#include <variant>
 
 // xxh64 of string "CANCEL_ERROR" generated at https://www.pelock.com/products/hash-calculator
 static constexpr UniqueKey BATCH_SENTINEL{ 0x2DDFEE0968C62AA7ull, "linda.batched" };
 
+// #################################################################################################
+
 class LindaFactory : public DeepFactory
 {
     private:
@@ -58,117 +59,84 @@ class LindaFactory : public DeepFactory
 static LindaFactory g_LindaFactory;
 
 // #################################################################################################
+// #################################################################################################
 
-/*
-* Actual data is kept within a keeper state, which is hashed by the 'Linda'
-* pointer (which is same to all userdatas pointing to it).
-*/
-class Linda : public DeepPrelude // Deep userdata MUST start with this header
+// Any hashing will do that maps pointers to [0..Universe::nb_keepers[ consistently.
+// Pointers are often aligned by 8 or so - ignore the low order bits
+// have to cast to unsigned long to avoid compilation warnings about loss of data when converting pointer-to-integer
+static constexpr uintptr_t KEEPER_MAGIC_SHIFT{ 3 };
+
+Linda::Linda(Universe* U_, LindaGroup group_, char const* name_, size_t len_)
+: DeepPrelude{ g_LindaFactory }
+, U{ U_ }
+, m_keeper_index{ (group_ ? group_ : static_cast<int>(std::bit_cast<uintptr_t>(this) >> KEEPER_MAGIC_SHIFT)) % U_->keepers->nb_keepers }
 {
-    private:
+    setName(name_, len_);
+}
 
-    static constexpr size_t kEmbeddedNameLength = 24;
-    using EmbeddedName = std::array<char, kEmbeddedNameLength>;
-    struct AllocatedName
-    {
-        size_t len{ 0 };
-        char* name{ nullptr };
-    };
-    // depending on the name length, it is either embedded inside the Linda, or allocated separately
-    std::variant<AllocatedName, EmbeddedName> m_name;
-
-    public:
-
-    std::condition_variable m_read_happened;
-    std::condition_variable m_write_happened;
-    Universe* const U; // the universe this linda belongs to
-    uintptr_t const group; // a group to control keeper allocation between lindas
-    CancelRequest simulate_cancel{ CancelRequest::None };
-
-    public:
-
-    // a fifo full userdata has one uservalue, the table that holds the actual fifo contents
-    [[nodiscard]] static void* operator new(size_t size_, Universe* U_) noexcept { return U_->internal_allocator.alloc(size_); }
-    // always embedded somewhere else or "in-place constructed" as a full userdata
-    // 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(void* p_, Universe* U_) { U_->internal_allocator.free(p_, sizeof(Linda)); }
-    // this one is for us, to make sure memory is freed by the correct allocator
-    static void operator delete(void* p_) { static_cast<Linda*>(p_)->U->internal_allocator.free(p_, sizeof(Linda)); }
-
-    Linda(Universe* U_, uintptr_t group_, char const* name_, size_t len_)
-    : DeepPrelude{ g_LindaFactory }
-    , U{ U_ }
-    , group{ group_ << KEEPER_MAGIC_SHIFT }
-    {
-        setName(name_, len_);
-    }
+// #################################################################################################
 
-    ~Linda()
+Linda::~Linda()
+{
+    if (std::holds_alternative<AllocatedName>(m_name))
     {
-        if (std::holds_alternative<AllocatedName>(m_name))
-        {
-            AllocatedName& name = std::get<AllocatedName>(m_name);
-            U->internal_allocator.free(name.name, name.len);
-        }
+        AllocatedName& name = std::get<AllocatedName>(m_name);
+        U->internal_allocator.free(name.name, name.len);
     }
+}
 
-    static int ProtectedCall(lua_State* L, lua_CFunction f_);
-
-    private :
+// #################################################################################################
 
-    void setName(char const* name_, size_t len_)
+void Linda::setName(char const* name_, size_t len_)
+{
+    // keep default
+    if (!name_ || len_ == 0)
     {
-        // keep default
-        if (!name_ || len_ == 0)
-        {
-            return;
-        }
-        ++len_; // don't forget terminating 0
-        if (len_ < kEmbeddedNameLength)
-        {
-            m_name.emplace<EmbeddedName>();
-            char* const name{ std::get<EmbeddedName>(m_name).data() };
-            memcpy(name, name_, len_);
-        }
-        else
-        {
-            AllocatedName& name = std::get<AllocatedName>(m_name);
-            name.name = static_cast<char*>(U->internal_allocator.alloc(len_));
-            name.len = len_;
-            memcpy(name.name, name_, len_);
-        }
+        return;
     }
+    ++len_; // don't forget terminating 0
+    if (len_ < kEmbeddedNameLength)
+    {
+        m_name.emplace<EmbeddedName>();
+        char* const name{ std::get<EmbeddedName>(m_name).data() };
+        memcpy(name, name_, len_);
+    }
+    else
+    {
+        AllocatedName& name = std::get<AllocatedName>(m_name);
+        name.name = static_cast<char*>(U->internal_allocator.alloc(len_));
+        name.len = len_;
+        memcpy(name.name, name_, len_);
+    }
+}
 
-    public:
-
-    uintptr_t hashSeed() const { return group ? group : std::bit_cast<uintptr_t>(this); }
+// #################################################################################################
 
-    char const* getName() const
+char const* Linda::getName() const
+{
+    if (std::holds_alternative<AllocatedName>(m_name))
     {
-        if (std::holds_alternative<AllocatedName>(m_name))
-        {
-            AllocatedName const& name = std::get<AllocatedName>(m_name);
-            return name.name;
-        }
-        if (std::holds_alternative<EmbeddedName>(m_name))
-        {
-            char const* const name{ std::get<EmbeddedName>(m_name).data() };
-            return name;
-        }
-        return nullptr;
+        AllocatedName const& name = std::get<AllocatedName>(m_name);
+        return name.name;
     }
-};
+    if (std::holds_alternative<EmbeddedName>(m_name))
+    {
+        char const* const name{ std::get<EmbeddedName>(m_name).data() };
+        return name;
+    }
+    return nullptr;
+}
 
 // #################################################################################################
 
 template <bool OPT>
-[[nodiscard]] static inline Linda* ToLinda(lua_State* L, int idx_)
+[[nodiscard]] static inline Linda* ToLinda(lua_State* L_, int idx_)
 {
-    Linda* const linda{ static_cast<Linda*>(g_LindaFactory.toDeep(L, idx_)) };
+    Linda* const linda{ static_cast<Linda*>(g_LindaFactory.toDeep(L_, idx_)) };
     if constexpr (!OPT)
     {
-        luaL_argcheck(L, linda != nullptr, idx_, "expecting a linda object");
-        LUA_ASSERT(L, linda->U == universe_get(L));
+        luaL_argcheck(L_, linda != nullptr, idx_, "expecting a linda object"); // doesn't return if linda is nullptr
+        LUA_ASSERT(L_, linda->U == universe_get(L_));
     }
     return linda;
 }
@@ -213,7 +181,7 @@ int Linda::ProtectedCall(lua_State* L, lua_CFunction f_)
     Linda* const linda{ ToLinda<false>(L, 1) };
 
     // acquire the keeper
-    Keeper* const K{ keeper_acquire(linda->U->keepers, linda->hashSeed()) };
+    Keeper* const K{ linda->acquireKeeper() };
     lua_State* const KL{ K ? K->L : nullptr };
     if (KL == nullptr)
         return 0;
@@ -229,7 +197,7 @@ int Linda::ProtectedCall(lua_State* L, lua_CFunction f_)
     lua_settop(KL, 0);
 
     // release the keeper
-    keeper_release(K);
+    linda->releaseKeeper(K);
 
     // if there was an error, forward it
     if (rc != LUA_OK)
@@ -306,7 +274,7 @@ LUAG_FUNC(linda_send)
         KeeperCallResult pushed;
         {
             Lane* const lane{ LANE_POINTER_REGKEY.readLightUserDataValue<Lane>(L) };
-            Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) };
+            Keeper* const K{ linda->whichKeeper() };
             KeeperState const KL{ K ? K->L : nullptr };
             if (KL == nullptr)
                 return 0;
@@ -472,7 +440,7 @@ LUAG_FUNC(linda_receive)
         }
 
         Lane* const lane{ LANE_POINTER_REGKEY.readLightUserDataValue<Lane>(L) };
-        Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) };
+        Keeper* const K{ linda->whichKeeper() };
         KeeperState const KL{ K ? K->L : nullptr };
         if (KL == nullptr)
             return 0;
@@ -584,7 +552,7 @@ LUAG_FUNC(linda_set)
         // make sure the key is of a valid type (throws an error if not the case)
         check_key_types(L, 2, 2);
 
-        Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) };
+        Keeper* const K{ linda->whichKeeper() };
         KeeperCallResult pushed;
         if (linda->simulate_cancel == CancelRequest::None)
         {
@@ -639,7 +607,7 @@ LUAG_FUNC(linda_count)
         // make sure the keys are of a valid type
         check_key_types(L, 2, lua_gettop(L));
 
-        Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) };
+        Keeper* const K{ linda->whichKeeper() };
         KeeperCallResult const pushed{ keeper_call(linda->U, K->L, KEEPER_API(count), L, linda, 2) };
         return OptionalValue(pushed, L, "tried to count an invalid key");
     };
@@ -667,7 +635,7 @@ LUAG_FUNC(linda_get)
         KeeperCallResult pushed;
         if (linda->simulate_cancel == CancelRequest::None)
         {
-            Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) };
+            Keeper* const K{ linda->whichKeeper() };
             pushed = keeper_call(linda->U, K->L, KEEPER_API(get), L, linda, 2);
             if (pushed.value_or(0) > 0)
             {
@@ -709,7 +677,7 @@ LUAG_FUNC(linda_limit)
         KeeperCallResult pushed;
         if (linda->simulate_cancel == CancelRequest::None)
         {
-            Keeper* const K{ which_keeper(linda->U->keepers, linda->hashSeed()) };
+            Keeper* const K{ linda->whichKeeper() };
             pushed = keeper_call(linda->U, K->L, KEEPER_API(limit), L, linda, 2);
             LUA_ASSERT(L, pushed.has_value() && (pushed.value() == 0 || pushed.value() == 1)); // no error, optional boolean value saying if we should wake blocked writer threads
             if (pushed.value() == 1)
@@ -863,7 +831,7 @@ LUAG_FUNC(linda_dump)
     auto dump = [](lua_State* L)
     {
         Linda* const linda{ ToLinda<false>(L, 1) };
-        return keeper_push_linda_storage(linda->U, DestState{ L }, linda, linda->hashSeed());
+        return keeper_push_linda_storage(*linda, DestState{ L });
     };
     return Linda::ProtectedCall(L, dump);
 }
@@ -871,13 +839,13 @@ LUAG_FUNC(linda_dump)
 // #################################################################################################
 
 /*
- * table = linda:dump()
- * return a table listing all pending data inside the linda
+ * table/string = linda:__towatch()
+ * return a table listing all pending data inside the linda, or the stringified linda if empty
  */
 LUAG_FUNC(linda_towatch)
 {
     Linda* const linda{ ToLinda<false>(L, 1) };
-    int pushed{ keeper_push_linda_storage(linda->U, DestState{ L }, linda, linda->hashSeed()) };
+    int pushed{ keeper_push_linda_storage(*linda, DestState{ L }) };
     if (pushed == 0)
     {
         // if the linda is empty, don't return nil
@@ -892,7 +860,7 @@ DeepPrelude* LindaFactory::newDeepObjectInternal(lua_State* L) const
 {
     size_t name_len = 0;
     char const* linda_name = nullptr;
-    unsigned long linda_group = 0;
+    LindaGroup linda_group{ 0 };
     // should have a string and/or a number of the stack as parameters (name and group)
     switch (lua_gettop(L))
     {
@@ -906,13 +874,13 @@ DeepPrelude* LindaFactory::newDeepObjectInternal(lua_State* L) const
         }
         else
         {
-            linda_group = (unsigned long) lua_tointeger(L, -1);
+            linda_group = LindaGroup{ static_cast<int>(lua_tointeger(L, -1)) };
         }
         break;
 
         case 2: // 2 parameters, a name and group, in that order
         linda_name = lua_tolstring(L, -2, &name_len);
-        linda_group = (unsigned long) lua_tointeger(L, -1);
+        linda_group = LindaGroup{ static_cast<int>(lua_tointeger(L, -1)) };
         break;
     }
 
@@ -932,7 +900,7 @@ void LindaFactory::deleteDeepObjectInternal(lua_State* L, DeepPrelude* o_) const
 {
     Linda* const linda{ static_cast<Linda*>(o_) };
     LUA_ASSERT(L, linda);
-    Keeper* const myK{ which_keeper(linda->U->keepers, linda->hashSeed()) };
+    Keeper* const myK{ linda->whichKeeper() };
     // if collected after the universe, keepers are already destroyed, and there is nothing to clear
     if (myK)
     {
@@ -940,13 +908,13 @@ void LindaFactory::deleteDeepObjectInternal(lua_State* L, DeepPrelude* o_) const
         // because we are already inside a protected area, and trying to do so would deadlock!
         bool const need_acquire_release{ myK->L != L };
         // Clean associated structures in the keeper state.
-        Keeper* const K{ need_acquire_release ? keeper_acquire(linda->U->keepers, linda->hashSeed()) : myK };
+        Keeper* const K{ need_acquire_release ? linda->acquireKeeper() : myK };
         // hopefully this won't ever raise an error as we would jump to the closest pcall site while forgetting to release the keeper mutex...
         [[maybe_unused]] KeeperCallResult const result{ keeper_call(linda->U, K->L, KEEPER_API(clear), L, linda, 0) };
         LUA_ASSERT(L, result.has_value() && result.value() == 0);
         if (need_acquire_release)
         {
-            keeper_release(K);
+            linda->releaseKeeper(K);
         }
     }
 
diff --git a/src/linda.h b/src/linda.h
new file mode 100644
index 0000000..559ef24
--- /dev/null
+++ b/src/linda.h
@@ -0,0 +1,70 @@
+#pragma once
+
+#include "cancel.h"
+#include "deep.h"
+#include "keeper.h"
+#include "universe.h"
+
+#include <array>
+#include <condition_variable>
+#include <variant>
+
+struct Keeper;
+
+// #################################################################################################
+
+using LindaGroup = Unique<int>;
+
+class Linda : public DeepPrelude // Deep userdata MUST start with this header
+{
+    private:
+
+    static constexpr size_t kEmbeddedNameLength = 24;
+    using EmbeddedName = std::array<char, kEmbeddedNameLength>;
+    struct AllocatedName
+    {
+        size_t len{ 0 };
+        char* name{ nullptr };
+    };
+    // depending on the name length, it is either embedded inside the Linda, or allocated separately
+    std::variant<AllocatedName, EmbeddedName> m_name;
+
+    public:
+
+    std::condition_variable m_read_happened;
+    std::condition_variable m_write_happened;
+    Universe* const U{ nullptr }; // the universe this linda belongs to
+    int const m_keeper_index{ -1 }; // the keeper associated to this linda
+    CancelRequest simulate_cancel{ CancelRequest::None };
+
+    public:
+
+    // a fifo full userdata has one uservalue, the table that holds the actual fifo contents
+    [[nodiscard]] static void* operator new(size_t size_, Universe* U_) noexcept { return U_->internal_allocator.alloc(size_); }
+    // always embedded somewhere else or "in-place constructed" as a full userdata
+    // 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(void* p_, Universe* U_) { U_->internal_allocator.free(p_, sizeof(Linda)); }
+    // this one is for us, to make sure memory is freed by the correct allocator
+    static void operator delete(void* p_) { static_cast<Linda*>(p_)->U->internal_allocator.free(p_, sizeof(Linda)); }
+
+    ~Linda();
+    Linda(Universe* U_, LindaGroup group_, char const* name_, size_t len_);
+    Linda() = delete;
+    Linda(Linda const&) = delete;
+    Linda(Linda const&&) = delete;
+    Linda& operator=(Linda const&) = delete;
+    Linda& operator=(Linda const&&) = delete;
+
+    static int ProtectedCall(lua_State* L, lua_CFunction f_);
+
+    private :
+
+    void setName(char const* name_, size_t len_);
+
+    public:
+
+    [[nodiscard]] char const* getName() const;
+    [[nodiscard]] Keeper* whichKeeper() const { return U->keepers->nb_keepers ? &U->keepers->keeper_array[m_keeper_index] : nullptr; }
+    [[nodiscard]] Keeper* acquireKeeper() const;
+    void releaseKeeper(Keeper* keeper_) const;
+};