Make Lanes crash on purpose at shutdown if some lanes still run

7 files changed, 48 insertions, 54 deletions

diff --git a/CHANGES b/CHANGES
index 59ad924..9b87eda 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,7 +9,6 @@ CHANGE 2: BGe 11-Jun-24
             - demote_full_userdata removed. Use __lanesconvert instead.
             - keepers_gc_threshold added. Controls when GC runs inside keepers.
             - nb_keepers changed to nb_user_keepers. limited to 100 keepers on top of the internal keeper used by the timer Linda.
-            - shutdown_mode added. Controls how free running lanes are signalled at Lanes shutdown.
             - strip_functions added. Only useful for Lua 5.3+.
             - verbose_errors removed. Use lane error_trace_level instead.
             - with_timers is false by default.
@@ -17,7 +16,8 @@ CHANGE 2: BGe 11-Jun-24
             - __lanesignore removed. Use __lanesconvert instead.
             - __lanesconvert added.
         - Lanes API and behavior:
-            - new function lanes.finally(). Installs a function that gets called at Lanes shutdown.
+            - new function lanes.finally(). Installs a function that gets called at Lanes shutdown after attempting to terminate all lanes.
+            - If some lanes still run at shutdown, Lanes with throw an exception (or freeze, this is to be decided).
             - lanes have a __close metamethod that calls join().
             - lanes can no longer be "killed" by hard-stopping their thread without any resource cleanup (see lane:cancel()).
             - lane:join() returns nil, error in case of problem.
diff --git a/docs/index.html b/docs/index.html
index 148b5ab..1cc007f 100644
--- a/docs/index.html
+++ b/docs/index.html
@@ -370,18 +370,6 @@
                 </tr>
 
                 <tr valign=top>
-                        <td id="shutdown_mode">
-                                <code>.shutdown_mode</code>
-                        </td>
-                        <td>
-                                <tt>"hard"</tt>/<tt>"soft"</tt>/<tt>"call"</tt>/<tt>"ret"</tt>/<tt>"line"</tt>/<tt>"count"</tt>
-                        </td>
-                        <td>
-                                Select the cancellation mode used at Lanes shutdown to request free running lane termination. See <a href="#cancelling">lane cancellation</a>. Default is <tt>"hard"</tt>.
-                        </td>
-                </tr>
-
-                <tr valign=top>
                         <td id="shutdown_timeout">
                                 <code>.shutdown_timeout</code>
                         </td>
@@ -389,7 +377,9 @@
                                 number >= 0
                         </td>
                         <td>
-                                Sets the duration in seconds Lanes will wait for graceful termination of running lanes at application shutdown. Default is <tt>0.25</tt>.
+                                Sets the duration in seconds Lanes will wait for graceful termination of running lanes at application shutdown. Default is <tt>0.25</tt>.<br />
+                                Lanes signals all lanes for cancellation with <tt>"soft"</tt>, <tt>"hard"</tt>, and <tt>"all"</tt> modes, in that order. Each attempt has <tt>shutdown_timeout</tt> seconds to succeed before the next one.<br />
+                                Then there is a last chance at cleanup with <a href="#finally"><tt>lanes.finally()</tt></a>. If some lanes are still running after that point, shutdown will freeze the application forever. It is YOUR responsibility to cleanup properly after yourself.
                         </td>
                 </tr>
 
@@ -462,7 +452,7 @@
         </tr>
 </table>
 
-<p>
+<p id="finally">
         It is also possible to install a function that will be called when Lanes is shutdown (that is, when the first state that required Lanes is closed).
 </p>
 
@@ -479,7 +469,8 @@
 <p>
         An error will be raised if you attempt to do this from inside a lane, or on bad arguments (non-function, or too many arguments).<br />
         Only the last registered finalizer is kept. It can be cleared by passing <tt>nil</tt> or nothing.<br />
-        The installed function is called after all free-running lanes are terminated, but before lindas become unusable.<br />
+        The installed function is called after all free-running lanes got a chance to terminate (see<a href="#shutdown_timeout"><tt>shutdown_timeout</tt></a>), but before lindas become unusable.<br />
+        The finalizer receives a single argument, a <tt>bool</tt> indicating whether some Lanes are still running or not at that point. It is possible to inspect them with <a href="#tracking">tracking</a>.<br />
         If an error occurs inside this finalizer, it is silently swallowed, since it happens only during state shutdown, and you can't do anything about it.
 </p>
 
@@ -1064,26 +1055,31 @@
         <br />
         <tt>cancel()</tt> sends a cancellation request to the lane.
         <br />
-        First argument is a <tt>mode</tt> can be one of <tt>"hard"</tt>, <tt>"soft"</tt>, <tt>"call"</tt>, <tt>"ret"</tt>, <tt>"line"</tt>, <tt>"count"</tt>.
+        First argument is a <tt>mode</tt> can be one of:
+        <ul>
+                <li>
+                        <tt>"soft"</tt>: Cancellation will only cause <tt>cancel_test()</tt> to return <tt>true</tt>, so that the lane can cleanup manually.
+                </li>
+                <li>
+                        <tt>"hard"</tt>: waits for the request to be processed, or a timeout to occur. <a href="#lindas">Linda</a> operations detecting the cancellation request will raise a special cancellation error (meaning they won't return in that case).<br />
+                        <tt>wake_lane</tt> defaults to <tt>true</tt>, and <tt>timeout</tt> defaults to 0 if not specified.
+                </li>
+                <li>
+                        <tt>"call"</tt>, <tt>"ret"</tt>, <tt>"line"</tt>, <tt>"count"</tt>: Asynchronously install the corresponding hook, then behave as <tt>"hard"</tt>.
+                </li>
+                <li>
+                        <tt>"all"</tt>: Installs all hooks in one shot, just to be sure.
+                </li>
+        </ul>
         If <tt>mode</tt> is not specified, it defaults to <tt>"hard"</tt>.
         If <tt>wake_lane</tt> is <tt>true</tt>, the lane is also signalled so that execution returns from any pending <a href="#lindas">Linda</a> operation. <a href="#lindas">Linda</a> operations detecting the cancellation request return <tt>lanes.cancel_error</tt>.
 </p>
 <p>
-        If <tt>mode</tt> is <tt>"soft"</tt>, cancellation will only cause <tt>cancel_test()</tt> to return <tt>true</tt>, so that the lane can cleanup manually.<br />
-</p>
-<p>
-        If <tt>mode</tt> is <tt>"hard"</tt>, waits for the request to be processed, or a timeout to occur. <a href="#lindas">Linda</a> operations detecting the cancellation request will raise a special cancellation error (meaning they won't return in that case).<br />
-        <tt>wake_lane</tt> defaults to <tt>true</tt>, and <tt>timeout</tt> defaults to 0 if not specified.
-</p>
-<p>
-        Other values of <tt>mode</tt> will asynchronously install the corresponding hook, then behave as <tt>"hard"</tt>.
-</p>
-<p>
         Returns <tt>true, lane_h.status</tt> if lane was already done (in <tt>"done"</tt>, <tt>"error"</tt> or <tt>"cancelled"</tt> status), or the cancellation was fruitful within <tt>timeout_secs</tt> timeout period.<br />
         Returns <tt>false, "timeout"</tt> otherwise.
 </p>
 <p>
-        If the lane is still running after the timeout expired, there is a chance lanes will raise an error at shutdown when failing to terminate all free-running lanes within the specified timeout.
+        If the lane is still running after the timeout expired, there is a chance lanes will freeze forever at shutdown when failing to terminate all free-running lanes within the specified timeout.
 </p>
 <p>
         Cancellation is tested <u>before</u> going to sleep in <tt>receive()</tt> or <tt>send()</tt> calls and after executing <tt>cancelstep</tt> Lua statements. A pending <tt>receive()</tt>or <tt>send()</tt> call is awakened.
diff --git a/src/cancel.cpp b/src/cancel.cpp
index 755215f..15a2c83 100644
--- a/src/cancel.cpp
+++ b/src/cancel.cpp
@@ -95,6 +95,8 @@ CancelOp WhichCancelOp(std::string_view const& opString_)
         _op = CancelOp::MaskLine;
     } else if (opString_ == "count") {
         _op = CancelOp::MaskCount;
+    } else if (opString_ == "all") {
+        _op = CancelOp::MaskAll;
     }
     return _op;
 }
diff --git a/src/cancel.h b/src/cancel.h
index 93fae4d..e62cf0a 100644
--- a/src/cancel.h
+++ b/src/cancel.h
@@ -29,6 +29,7 @@ enum class CancelOp
     MaskRet = LUA_MASKRET,
     MaskLine = LUA_MASKLINE,
     MaskCount = LUA_MASKCOUNT,
+    MaskAll = LUA_MASKCALL | LUA_MASKRET | LUA_MASKLINE | LUA_MASKCOUNT
 };
 
 // xxh64 of string "kCancelError" generated at https://www.pelock.com/products/hash-calculator
diff --git a/src/lanes.lua b/src/lanes.lua
index 48ebeb6..d28fcf4 100644
--- a/src/lanes.lua
+++ b/src/lanes.lua
@@ -98,7 +98,6 @@ local default_params =
     keepers_gc_threshold = -1,
     nb_user_keepers = 0,
     on_state_create = nil,
-    shutdown_mode = "hard",
     shutdown_timeout = 0.25,
     strip_functions = true,
     track_lanes = false,
@@ -159,14 +158,6 @@ local param_checkers =
         end
         return true
     end,
-    shutdown_mode = function(val_)
-        local valid_hooks = { soft = true, hard = true, call = true, ret = true, line = true, count = true }
-        -- shutdown_mode should be a known hook mask
-        if not valid_hooks[val_] then
-            return nil, "unknown value"
-        end
-        return true
-    end,
     shutdown_timeout = function(val_)
         -- shutdown_timeout should be a number in [0,3600]
         if type(val_) ~= "number" then
diff --git a/src/universe.cpp b/src/universe.cpp
index 1cb4fd0..5fda29a 100644
--- a/src/universe.cpp
+++ b/src/universe.cpp
@@ -147,8 +147,7 @@ void Universe::callOnStateCreate(lua_State* const L_, lua_State* const from_, Lo
     DEBUGSPEW_CODE(DebugSpewIndentScope _scope{ _U });
     lua_createtable(L_, 0, 1);                                                                     // L_: settings universe {mt}
     std::ignore = luaG_getfield(L_, 1, "shutdown_timeout");                                        // L_: settings universe {mt} shutdown_timeout
-    std::ignore = luaG_getfield(L_, 1, "shutdown_mode");                                           // L_: settings universe {mt} shutdown_timeout shutdown_mode
-    lua_pushcclosure(L_, LG_universe_gc, 2);                                                       // L_: settings universe {mt} LG_universe_gc
+    lua_pushcclosure(L_, LG_universe_gc, 1);                                                       // L_: settings universe {mt} LG_universe_gc
     lua_setfield(L_, -2, "__gc");                                                                  // L_: settings universe {mt}
     lua_setmetatable(L_, -2);                                                                      // L_: settings universe
     lua_pop(L_, 1);                                                                                // L_: settings
@@ -352,7 +351,7 @@ lanes::AllocatorDefinition Universe::resolveAllocator(lua_State* const L_, std::
 
 // #################################################################################################
 
-void Universe::terminateFreeRunningLanes(lua_State* const L_, lua_Duration const shutdownTimeout_, CancelOp const op_)
+bool Universe::terminateFreeRunningLanes(lua_Duration const shutdownTimeout_, CancelOp const op_)
 {
     if (selfdestructFirst != SELFDESTRUCT_END) {
         // Signal _all_ still running threads to exit (including the timer thread)
@@ -404,15 +403,8 @@ void Universe::terminateFreeRunningLanes(lua_State* const L_, lua_Duration const
         }
     }
 
-    // If after all this, we still have some free-running lanes, it's an external user error, they should have stopped appropriately
-    {
-        std::lock_guard<std::mutex> _guard{ selfdestructMutex };
-        Lane* _lane{ selfdestructFirst };
-        if (_lane != SELFDESTRUCT_END) {
-            // this causes a leak because we don't call U's destructor (which could be bad if the still running lanes are accessing it)
-            raise_luaL_error(L_, "Zombie thread '%s' refuses to die!", _lane->debugName.data());
-        }
-    }
+    // are all lanes successfully terminated?
+    return selfdestructFirst == SELFDESTRUCT_END;
 }
 
 // #################################################################################################
@@ -421,15 +413,21 @@ void Universe::terminateFreeRunningLanes(lua_State* const L_, lua_Duration const
 LUAG_FUNC(universe_gc)
 {
     lua_Duration const _shutdown_timeout{ lua_tonumber(L_, lua_upvalueindex(1)) };
-    std::string_view const _op_string{ luaG_tostring(L_, lua_upvalueindex(2)) };
     STACK_CHECK_START_ABS(L_, 1);
     Universe* const _U{ luaG_tofulluserdata<Universe>(L_, 1) };                                    // L_: U
-    _U->terminateFreeRunningLanes(L_, _shutdown_timeout, WhichCancelOp(_op_string));
+
+    // attempt to terminate all lanes with increasingly stronger cancel methods
+    bool const _allLanesTerminated{ 
+        _U->terminateFreeRunningLanes(_shutdown_timeout, CancelOp::Soft)
+        || _U->terminateFreeRunningLanes(_shutdown_timeout, CancelOp::Hard)
+        || _U->terminateFreeRunningLanes(_shutdown_timeout, CancelOp::MaskAll)
+    };
 
     // invoke the function installed by lanes.finally()
     kFinalizerRegKey.pushValue(L_);                                                                // L_: U finalizer|nil
     if (!lua_isnil(L_, -1)) {
-        lua_pcall(L_, 0, 0, 0);                                                                    // L_: U
+        lua_pushboolean(L_, _allLanesTerminated);                                                  // L_: U finalizer bool
+        lua_pcall(L_, 1, 0, 0);                                                                    // L_: U
         // discard any error that might have occured
         lua_settop(L_, 1);
     } else {
@@ -438,6 +436,12 @@ LUAG_FUNC(universe_gc)
     // in case of error, the message is pushed on the stack
     STACK_CHECK(L_, 1);
 
+    // if some lanes are still running here, we have no other choice than crashing and let the client figure out what's wrong
+    while (_U->selfdestructFirst != SELFDESTRUCT_END) {
+        throw std::logic_error{ "Some lanes are still running at shutdown" };
+        //std::this_thread::yield();
+    }
+
     // no need to mutex-protect this as all threads in the universe are gone at that point
     if (_U->timerLinda != nullptr) { // test in case some early internal error prevented Lanes from creating the deep timer
         [[maybe_unused]] int const _prev_ref_count{ _U->timerLinda->refcount.fetch_sub(1, std::memory_order_relaxed) };
diff --git a/src/universe.h b/src/universe.h
index 6374648..dc8940f 100644
--- a/src/universe.h
+++ b/src/universe.h
@@ -139,7 +139,7 @@ class Universe
     void initializeOnStateCreate(lua_State* const L_);
     lanes::AllocatorDefinition resolveAllocator(lua_State* const L_, std::string_view const& hint_) const;
     static inline void Store(lua_State* L_, Universe* U_);
-    void terminateFreeRunningLanes(lua_State* L_, lua_Duration shutdownTimeout_, CancelOp op_);
+    [[nodiscard]] bool terminateFreeRunningLanes(lua_Duration shutdownTimeout_, CancelOp op_);
 };
 
 // #################################################################################################