From 740b820e429cf7c856f2f305b6fc5b6fd969f3b3 Mon Sep 17 00:00:00 2001
From: Benoit Germain <benoit.germain@ubisoft.com>
Date: Tue, 11 Jun 2024 17:24:30 +0200
Subject: Move some cancel-related code around

---
 src/cancel.cpp   | 113 ++++++++++++-------------------------------------------
 src/cancel.h     |   4 +-
 src/lane.cpp     |  65 ++++++++++++++++++++++++++++++++
 src/lane.h       |  10 ++++-
 src/universe.cpp |   4 +-
 5 files changed, 103 insertions(+), 93 deletions(-)

diff --git a/src/cancel.cpp b/src/cancel.cpp
index 4205d1f..a62fcdf 100644
--- a/src/cancel.cpp
+++ b/src/cancel.cpp
@@ -50,40 +50,13 @@ THE SOFTWARE.
  * Returns CANCEL_SOFT/HARD if any locks are to be exited, and 'raise_cancel_error()' called,
  * to make execution of the lane end.
  */
-[[nodiscard]] static inline CancelRequest cancel_test(lua_State* L_)
+[[nodiscard]] CancelRequest cancel_test(lua_State* const L_)
 {
     Lane* const _lane{ kLanePointerRegKey.readLightUserDataValue<Lane>(L_) };
     // 'lane' is nullptr for the original main state (and no-one can cancel that)
     return _lane ? _lane->cancelRequest : CancelRequest::None;
 }
 
-// #################################################################################################
-
-//---
-// bool = cancel_test()
-//
-// Available inside the global namespace of lanes
-// returns a boolean saying if a cancel request is pending
-//
-LUAG_FUNC(cancel_test)
-{
-    CancelRequest _test{ cancel_test(L_) };
-    lua_pushboolean(L_, _test != CancelRequest::None);
-    return 1;
-}
-
-// #################################################################################################
-// #################################################################################################
-
-[[nodiscard]] static void cancel_hook(lua_State* L_, [[maybe_unused]] lua_Debug* ar_)
-{
-    DEBUGSPEW_CODE(DebugSpew(nullptr) << "cancel_hook" << std::endl);
-    if (cancel_test(L_) != CancelRequest::None) {
-        lua_sethook(L_, nullptr, 0, 0);
-        raise_cancel_error(L_);
-    }
-}
-
 // #################################################################################################
 // #################################################################################################
 
@@ -105,64 +78,9 @@ LUAG_FUNC(cancel_test)
 //
 
 // #################################################################################################
-
-[[nodiscard]] static CancelResult thread_cancel_soft(Lane* lane_, std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_)
-{
-    lane_->cancelRequest = CancelRequest::Soft; // it's now signaled to stop
-    // negative timeout: we don't want to truly abort the lane, we just want it to react to cancel_test() on its own
-    if (wakeLane_) { // wake the thread so that execution returns from any pending linda operation if desired
-        std::condition_variable* const _waiting_on{ lane_->waiting_on };
-        if (lane_->status == Lane::Waiting && _waiting_on != nullptr) {
-            _waiting_on->notify_all();
-        }
-    }
-
-    return lane_->waitForCompletion(until_) ? CancelResult::Cancelled : CancelResult::Timeout;
-}
-
 // #################################################################################################
 
-[[nodiscard]] static CancelResult thread_cancel_hard(Lane* lane_, std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_)
-{
-    lane_->cancelRequest = CancelRequest::Hard; // it's now signaled to stop
-    // lane_->thread.get_stop_source().request_stop();
-    if (wakeLane_) { // wake the thread so that execution returns from any pending linda operation if desired
-        std::condition_variable* const _waiting_on{ lane_->waiting_on };
-        if (lane_->status == Lane::Waiting && _waiting_on != nullptr) {
-            _waiting_on->notify_all();
-        }
-    }
-
-    CancelResult result{ lane_->waitForCompletion(until_) ? CancelResult::Cancelled : CancelResult::Timeout };
-    return result;
-}
-
-// #################################################################################################
-
-CancelResult thread_cancel(Lane* lane_, CancelOp op_, int hookCount_, std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_)
-{
-    // remember that lanes are not transferable: only one thread can cancel a lane, so no multithreading issue here
-    // We can read 'lane_->status' without locks, but not wait for it (if Posix no PTHREAD_TIMEDJOIN)
-    if (lane_->status >= Lane::Done) {
-        // say "ok" by default, including when lane is already done
-        return CancelResult::Cancelled;
-    }
-
-    // signal the linda the wake up the thread so that it can react to the cancel query
-    // let us hope we never land here with a pointer on a linda that has been destroyed...
-    if (op_ == CancelOp::Soft) {
-        return thread_cancel_soft(lane_, until_, wakeLane_);
-    } else if (static_cast<int>(op_) > static_cast<int>(CancelOp::Soft)) {
-        lua_sethook(lane_->L, cancel_hook, static_cast<int>(op_), hookCount_);
-    }
-
-    return thread_cancel_hard(lane_, until_, wakeLane_);
-}
-
-// #################################################################################################
-// #################################################################################################
-
-CancelOp which_cancel_op(std::string_view const& opString_)
+CancelOp WhichCancelOp(std::string_view const& opString_)
 {
     CancelOp _op{ CancelOp::Invalid };
     if (opString_ == "hard") {
@@ -183,11 +101,11 @@ CancelOp which_cancel_op(std::string_view const& opString_)
 
 // #################################################################################################
 
-[[nodiscard]] static CancelOp which_cancel_op(lua_State* L_, int idx_)
+[[nodiscard]] static CancelOp WhichCancelOp(lua_State* const L_, int const idx_)
 {
     if (luaG_type(L_, idx_) == LuaType::STRING) {
         std::string_view const _str{ luaG_tostring(L_, idx_) };
-        CancelOp _op{ which_cancel_op(_str) };
+        CancelOp _op{ WhichCancelOp(_str) };
         lua_remove(L_, idx_); // argument is processed, remove it
         if (_op == CancelOp::Invalid) {
             raise_luaL_error(L_, "invalid hook option %s", _str);
@@ -197,13 +115,32 @@ CancelOp which_cancel_op(std::string_view const& opString_)
     return CancelOp::Hard;
 }
 
+// #################################################################################################
+// #################################################################################################
+// ######################################### Lua API ###############################################
+// #################################################################################################
+// #################################################################################################
+
+//---
+// bool = cancel_test()
+//
+// Available inside the global namespace of a lane
+// returns a boolean saying if a cancel request is pending
+//
+LUAG_FUNC(cancel_test)
+{
+    CancelRequest _test{ cancel_test(L_) };
+    lua_pushboolean(L_, _test != CancelRequest::None);
+    return 1;
+}
+
 // #################################################################################################
 
 // bool[,reason] = lane_h:cancel( [mode, hookcount] [, timeout] [, wake_lane])
 LUAG_FUNC(thread_cancel)
 {
     Lane* const _lane{ ToLane(L_, 1) };
-    CancelOp const _op{ which_cancel_op(L_, 2) }; // this removes the op string from the stack
+    CancelOp const _op{ WhichCancelOp(L_, 2) }; // this removes the op string from the stack
 
     int _hook_count{ 0 };
     if (static_cast<int>(_op) > static_cast<int>(CancelOp::Soft)) { // hook is requested
@@ -237,7 +174,7 @@ LUAG_FUNC(thread_cancel)
         lua_remove(L_, 2); // argument is processed, remove it
     }
     STACK_CHECK_START_REL(L_, 0);
-    switch (thread_cancel(_lane, _op, _hook_count, _until, _wake_lane)) {
+    switch (_lane->cancel(_op, _hook_count, _until, _wake_lane)) {
     default: // should never happen unless we added a case and forgot to handle it
         LUA_ASSERT(L_, false);
         break;
diff --git a/src/cancel.h b/src/cancel.h
index 3304e0d..0e4e6d4 100644
--- a/src/cancel.h
+++ b/src/cancel.h
@@ -38,8 +38,8 @@ enum class CancelOp
 // xxh64 of string "kCancelError" generated at https://www.pelock.com/products/hash-calculator
 static constexpr UniqueKey kCancelError{ 0x0630345FEF912746ull, "lanes.cancel_error" }; // 'raise_cancel_error' sentinel
 
-[[nodiscard]] CancelOp which_cancel_op(std::string_view const& opString_);
-[[nodiscard]] CancelResult thread_cancel(Lane* lane_, CancelOp op_, int hookCount_, std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_);
+[[nodiscard]] CancelRequest cancel_test(lua_State* L_);
+[[nodiscard]] CancelOp WhichCancelOp(std::string_view const& opString_);
 
 [[noreturn]] static inline void raise_cancel_error(lua_State* L_)
 {
diff --git a/src/lane.cpp b/src/lane.cpp
index 5e018a4..15499fe 100644
--- a/src/lane.cpp
+++ b/src/lane.cpp
@@ -38,9 +38,11 @@ static constexpr UniqueKey kCachedError{ 0xD6F35DD608D0A203ull };
 // xxh64 of string "tostring" generated at https://www.pelock.com/products/hash-calculator
 static constexpr UniqueKey kCachedTostring{ 0xAB5EA23BCEA0C35Cull };
 
+// #################################################################################################
 // #################################################################################################
 // ######################################### Lua API ###############################################
 // #################################################################################################
+// #################################################################################################
 
 static LUAG_FUNC(get_debug_threadname)
 {
@@ -830,6 +832,69 @@ Lane::~Lane()
 
 // #################################################################################################
 
+CancelResult Lane::cancel(CancelOp op_, int hookCount_, std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_)
+{
+    auto _cancel_hook = +[](lua_State* const L_, [[maybe_unused]] lua_Debug* const ar_) {
+        DEBUGSPEW_CODE(DebugSpew(nullptr) << "cancel_hook" << std::endl);
+        if (cancel_test(L_) != CancelRequest::None) {
+            lua_sethook(L_, nullptr, 0, 0);
+            raise_cancel_error(L_);
+        }
+    };
+
+    // remember that lanes are not transferable: only one thread can cancel a lane, so no multithreading issue here
+    // We can read 'lane_->status' without locks, but not wait for it (if Posix no PTHREAD_TIMEDJOIN)
+    if (status >= Lane::Done) {
+        // say "ok" by default, including when lane is already done
+        return CancelResult::Cancelled;
+    }
+
+    // signal the linda the wake up the thread so that it can react to the cancel query
+    // let us hope we never land here with a pointer on a linda that has been destroyed...
+    if (op_ == CancelOp::Soft) {
+        return cancelSoft(until_, wakeLane_);
+    } else if (static_cast<int>(op_) > static_cast<int>(CancelOp::Soft)) {
+        lua_sethook(L, _cancel_hook, static_cast<int>(op_), hookCount_);
+    }
+
+    return cancelHard(until_, wakeLane_);
+}
+
+// #################################################################################################
+
+[[nodiscard]] CancelResult Lane::cancelHard(std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_)
+{
+    cancelRequest = CancelRequest::Hard; // it's now signaled to stop
+    // lane_->thread.get_stop_source().request_stop();
+    if (wakeLane_) { // wake the thread so that execution returns from any pending linda operation if desired
+        std::condition_variable* const _waiting_on{ waiting_on };
+        if (status == Lane::Waiting && _waiting_on != nullptr) {
+            _waiting_on->notify_all();
+        }
+    }
+
+    CancelResult result{ waitForCompletion(until_) ? CancelResult::Cancelled : CancelResult::Timeout };
+    return result;
+}
+
+// #################################################################################################
+
+[[nodiscard]] CancelResult Lane::cancelSoft(std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_)
+{
+    cancelRequest = CancelRequest::Soft; // it's now signaled to stop
+    // negative timeout: we don't want to truly abort the lane, we just want it to react to cancel_test() on its own
+    if (wakeLane_) { // wake the thread so that execution returns from any pending linda operation if desired
+        std::condition_variable* const _waiting_on{ waiting_on };
+        if (status == Lane::Waiting && _waiting_on != nullptr) {
+            _waiting_on->notify_all();
+        }
+    }
+
+    return waitForCompletion(until_) ? CancelResult::Cancelled : CancelResult::Timeout;
+}
+
+// #################################################################################################
+
 void Lane::changeDebugName(int const nameIdx_)
 {
     int const _nameIdx{ luaG_absindex(L, nameIdx_) };
diff --git a/src/lane.h b/src/lane.h
index 6146df8..7c75fe8 100644
--- a/src/lane.h
+++ b/src/lane.h
@@ -120,6 +120,14 @@ class Lane
     Lane(Universe* U_, lua_State* L_, ErrorTraceLevel errorTraceLevel_);
     ~Lane();
 
+    private:
+
+    [[nodiscard]] CancelResult cancelHard(std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_);
+    [[nodiscard]] CancelResult cancelSoft(std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_);
+
+    public:
+
+    CancelResult cancel(CancelOp op_, int hookCount_, std::chrono::time_point<std::chrono::steady_clock> until_, bool wakeLane_);
     void changeDebugName(int const nameIdx_);
     void close() { lua_State* _L{ L }; L = nullptr; lua_close(_L); }
     [[nodiscard]] std::string_view errorTraceLevelString() const;
@@ -137,7 +145,7 @@ class Lane
 
 // To allow free-running threads (longer lifespan than the handle's)
 // 'Lane' are malloc/free'd and the handle only carries a pointer.
-// This is not deep userdata since the handle's not portable among lanes.
+// This is not deep userdata since the handle is not portable among lanes.
 //
 [[nodiscard]] inline Lane* ToLane(lua_State* L_, int i_)
 {
diff --git a/src/universe.cpp b/src/universe.cpp
index 82522f1..c95884f 100644
--- a/src/universe.cpp
+++ b/src/universe.cpp
@@ -256,7 +256,7 @@ void Universe::terminateFreeRunningLanes(lua_State* const L_, lua_Duration const
                 // if waiting on a linda, they will raise a cancel_error.
                 // if a cancellation hook is desired, it will be installed to try to raise an error
                 if (_lane->thread.joinable()) {
-                    std::ignore = thread_cancel(_lane, op_, 1, std::chrono::steady_clock::now() + 1us, true);
+                    std::ignore = _lane->cancel(op_, 1, std::chrono::steady_clock::now() + 1us, true);
                 }
                 _lane = _lane->selfdestruct_next;
             }
@@ -316,7 +316,7 @@ LUAG_FUNC(universe_gc)
     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, which_cancel_op(_op_string));
+    _U->terminateFreeRunningLanes(L_, _shutdown_timeout, WhichCancelOp(_op_string));
 
     // invoke the function installed by lanes.finally()
     kFinalizerRegKey.pushValue(L_);                                                                // L_: U finalizer|nil
-- 
cgit v1.2.3-55-g6feb