From a86327245eea5638b933aec795026699201c19e1 Mon Sep 17 00:00:00 2001
From: Benoit Germain <benoit.germain@ubisoft.com>
Date: Mon, 28 Apr 2025 11:17:48 +0200
Subject: Minor internal tweaks in shutdown code

---
 src/keeper.cpp   |  9 +++++----
 src/keeper.hpp   |  2 +-
 src/lane.cpp     | 12 ++++++++----
 src/universe.cpp | 12 +++++++++---
 4 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/keeper.cpp b/src/keeper.cpp
index c8c470f..ca2a80d 100644
--- a/src/keeper.cpp
+++ b/src/keeper.cpp
@@ -952,15 +952,15 @@ void Keepers::collectGarbage()
 // #################################################################################################
 
 
-void Keepers::close()
+bool Keepers::close()
 {
     if (isClosing.test_and_set(std::memory_order_release)) {
-        assert(false); // should never close more than once in practice
-        return;
+        return false; // should never close more than once in practice
     }
 
+    // We may have not initialized the keepers if an error was raised in Universe::Create because of bad settings
     if (std::holds_alternative<std::monostate>(keeper_array)) {
-        return;
+        return true;
     }
 
     auto _closeOneKeeper = [](Keeper& keeper_) {
@@ -989,6 +989,7 @@ void Keepers::close()
     }
 
     keeper_array.emplace<std::monostate>();
+    return true;
 }
 
 // #################################################################################################
diff --git a/src/keeper.hpp b/src/keeper.hpp
index f1083b3..0aa44f2 100644
--- a/src/keeper.hpp
+++ b/src/keeper.hpp
@@ -68,7 +68,7 @@ struct Keepers
 
     Keepers() = default;
     void collectGarbage();
-    void close();
+    bool close();
     [[nodiscard]]
     Keeper* getKeeper(KeeperIndex idx_);
     [[nodiscard]]
diff --git a/src/lane.cpp b/src/lane.cpp
index 5cebdfa..e6ea2e5 100644
--- a/src/lane.cpp
+++ b/src/lane.cpp
@@ -783,10 +783,14 @@ static void lane_main(Lane* const lane_)
         if (lane_->selfdestructRemove()) { // check and remove (under lock!)
             // We're a free-running thread and no-one is there to clean us up.
             lane_->closeState();
-            lane_->U->selfdestructMutex.lock();
-            // done with lua_close(), terminal shutdown sequence may proceed
-            lane_->U->selfdestructingCount.fetch_sub(1, std::memory_order_release);
-            lane_->U->selfdestructMutex.unlock();
+
+            // let's try not to crash if the lane didn't terminate gracefully and the Universe met its end
+            if (!lane_->flaggedAfterUniverseGC.load(std::memory_order_relaxed)) {
+                lane_->U->selfdestructMutex.lock();
+                // done with lua_close(), terminal shutdown sequence may proceed
+                lane_->U->selfdestructingCount.fetch_sub(1, std::memory_order_release);
+                lane_->U->selfdestructMutex.unlock();
+            }
 
             // we destroy ourselves, therefore our thread member too, from inside the thread body
             // detach so that we don't try to join, as this doesn't seem a good idea
diff --git a/src/universe.cpp b/src/universe.cpp
index 335f056..0f41585 100644
--- a/src/universe.cpp
+++ b/src/universe.cpp
@@ -178,8 +178,8 @@ Universe* Universe::Create(lua_State* const L_)
 
     // Linked chains handling
     _U->selfdestructFirst = SELFDESTRUCT_END;
-    _U->initializeAllocatorFunction(L_);
-    _U->initializeOnStateCreate(L_);
+    _U->initializeAllocatorFunction(L_); // this can raise an error
+    _U->initializeOnStateCreate(L_); // this can raise an error
     _U->keepers.initialize(*_U, L_, static_cast<size_t>(_nbUserKeepers), _keepers_gc_threshold);
     STACK_CHECK(L_, 0);
 
@@ -463,7 +463,11 @@ int Universe::UniverseGC(lua_State* const L_)
             // that manifests as a crash inside ntdll!longjmp() function, in optimized builds only
             lua_error(L_);
         }
+    } else {
+        // we didn't use the error message, let's keep a clean stack
+        lua_pop(L_, 1);                                                                            // L_: U
     }
+    STACK_CHECK(L_, 1);
 
     // ---------------------------------------------------------
     // we don't reach that point if some lanes are still running
@@ -472,7 +476,9 @@ int Universe::UniverseGC(lua_State* const L_)
     // no need to mutex-protect this as all lanes in the universe are gone at that point
     Linda::DeleteTimerLinda(L_, std::exchange(_U->timerLinda, nullptr), PK);
 
-    _U->keepers.close();
+    if (!_U->keepers.close()) {
+        raise_luaL_error(L_, "INTERNAL ERROR: Keepers closed more than once");
+    }
 
     // remove the protected allocator, if any
     _U->protectedAllocator.removeFrom(L_);
-- 
cgit v1.2.3-55-g6feb