From acb203829bfd12b4cc0c90f82cb70a4d111bce46 Mon Sep 17 00:00:00 2001 From: Benoit Germain Date: Fri, 29 Mar 2024 12:11:56 +0100 Subject: C++ migration: Lane is a proper class with overloaded operator new/delete --- src/cancel.cpp | 4 +-- src/cancel.h | 2 +- src/lanes.cpp | 88 +++++++++++++++++++++++++---------------------------- src/lanes_private.h | 45 ++++++++++++++++++--------- src/universe.h | 2 +- 5 files changed, 76 insertions(+), 65 deletions(-) diff --git a/src/cancel.cpp b/src/cancel.cpp index 0f7a6af..07c316a 100644 --- a/src/cancel.cpp +++ b/src/cancel.cpp @@ -162,7 +162,7 @@ static CancelResult thread_cancel_hard(lua_State* L, Lane* lane_, double secs_, (void) waitkill_timeout_; // unused (void) L; // unused #endif // THREADAPI == THREADAPI_PTHREAD - lane_->mstatus = ThreadStatus::Killed; // mark 'gc' to wait for it + lane_->mstatus = Lane::Killed; // mark 'gc' to wait for it // note that lane_->status value must remain to whatever it was at the time of the kill // because we need to know if we can lua_close() the Lua State or not. result = CancelResult::Killed; @@ -176,7 +176,7 @@ CancelResult thread_cancel(lua_State* L, Lane* lane_, CancelOp op_, double secs_ { // 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_->mstatus == ThreadStatus::Killed) + if (lane_->mstatus == Lane::Killed) { return CancelResult::Killed; } diff --git a/src/cancel.h b/src/cancel.h index 481cca4..0c1de14 100644 --- a/src/cancel.h +++ b/src/cancel.h @@ -15,7 +15,7 @@ extern "C" { // ################################################################################################ -struct Lane; // forward +class Lane; // forward /* * Lane cancellation request modes diff --git a/src/lanes.cpp b/src/lanes.cpp index 327c2df..079b880 100644 --- a/src/lanes.cpp +++ b/src/lanes.cpp @@ -99,6 +99,27 @@ THE SOFTWARE. # include #endif +// forwarding (will do things better later) +static void tracking_add(Lane* lane_); + +Lane::Lane(Universe* U_, lua_State* L_) +: U{ U_ } +, L{ L_ } +{ +#if THREADWAIT_METHOD == THREADWAIT_CONDVAR + MUTEX_INIT(&done_lock); + SIGNAL_INIT(&done_signal); +#endif // THREADWAIT_METHOD == THREADWAIT_CONDVAR + +#if HAVE_LANE_TRACKING() + if (U->tracking_first) + { + tracking_add(this); + } +#endif // HAVE_LANE_TRACKING() +} + + /* Do you want full call stacks, or just the line where the error happened? * * TBD: The full stack feature does not seem to work (try 'make error'). @@ -228,27 +249,22 @@ static bool tracking_remove(Lane* lane_) // ################################################################################################# -//--- -// low-level cleanup - -static void lane_cleanup(Lane* lane_) +Lane::~Lane() { // Clean up after a (finished) thread // #if THREADWAIT_METHOD == THREADWAIT_CONDVAR - SIGNAL_FREE(&lane_->done_signal); - MUTEX_FREE(&lane_->done_lock); + SIGNAL_FREE(&done_signal); + MUTEX_FREE(&done_lock); #endif // THREADWAIT_METHOD == THREADWAIT_CONDVAR #if HAVE_LANE_TRACKING() - if (lane_->U->tracking_first != nullptr) + if (U->tracking_first != nullptr) { // Lane was cleaned up, no need to handle at process termination - tracking_remove(lane_); + tracking_remove(this); } #endif // HAVE_LANE_TRACKING() - - lane_->U->internal_allocator.free(lane_, sizeof(Lane)); } /* @@ -536,7 +552,7 @@ static int selfdestruct_gc( lua_State* L) #endif // THREADAPI == THREADAPI_PTHREAD } // NO lua_close() in this case because we don't know where execution of the state was interrupted - lane_cleanup(lane); + delete lane; lane = next_s; ++n; } @@ -565,7 +581,7 @@ static int selfdestruct_gc( lua_State* L) U->timer_deep = nullptr; } - close_keepers( U); + close_keepers(U); // remove the protected allocator, if any U->protected_allocator.removeFrom(L); @@ -919,7 +935,7 @@ static THREAD_RETURN_T THREAD_CALLCONV lane_main(void* vs) // STACK_DUMP(L); // Call finalizers, if the script has set them up. // - int rc2 = run_finalizers(L, rc); + int rc2{ run_finalizers(L, rc) }; DEBUGSPEW_CODE(fprintf(stderr, INDENT_BEGIN "Lane %p finalizer: %s\n" INDENT_END, L, get_errcode_name(rc2))); if (rc2 != LUA_OK) // Error within a finalizer! { @@ -938,7 +954,7 @@ static THREAD_RETURN_T THREAD_CALLCONV lane_main(void* vs) --lane->U->selfdestructing_count; lane->U->selfdestruct_cs.unlock(); - lane_cleanup(lane); // s is freed at this point + delete lane; } else { @@ -1207,35 +1223,14 @@ LUAG_FUNC( lane_new) STACK_CHECK( L2, 1 + nargs); // 'lane' is allocated from heap, not Lua, since its life span may surpass the handle's (if free running thread) - // - // a Lane full userdata needs a single uservalue - Lane** const ud{ lua_newuserdatauv(L, 1) }; // func libs priority globals package required gc_cb lane - Lane* const lane{ *ud = static_cast(U->internal_allocator.alloc(sizeof(Lane))) }; // don't forget to store the pointer in the userdata! + Lane* const lane{ new (U) Lane{ U, L2 } }; if (lane == nullptr) { return luaL_error(L, "could not create lane: out of memory"); } - - lane->L = L2; - lane->U = U; - lane->status = PENDING; - lane->waiting_on = nullptr; - lane->debug_name = ""; - lane->cancel_request = CancelRequest::None; - -#if THREADWAIT_METHOD == THREADWAIT_CONDVAR - MUTEX_INIT(&lane->done_lock); - SIGNAL_INIT(&lane->done_signal); -#endif // THREADWAIT_METHOD == THREADWAIT_CONDVAR - lane->mstatus = ThreadStatus::Normal; - lane->selfdestruct_next = nullptr; -#if HAVE_LANE_TRACKING() - lane->tracking_next = nullptr; - if (lane->U->tracking_first) - { - tracking_add(lane); - } -#endif // HAVE_LANE_TRACKING() + // a Lane full userdata needs a single uservalue + Lane** const ud{ lua_newuserdatauv(L, 1) }; // func libs priority globals package required gc_cb lane + *ud = lane; // don't forget to store the pointer in the userdata! // Set metatable for the userdata // @@ -1245,7 +1240,7 @@ LUAG_FUNC( lane_new) // Create uservalue for the userdata // (this is where lane body return values will be stored when the handle is indexed by a numeric key) - lua_newtable( L); // func libs cancelstep priority globals package required gc_cb lane uv + lua_newtable(L); // func libs cancelstep priority globals package required gc_cb lane uv // Store the gc_cb callback in the uservalue if (gc_cb_idx > 0) @@ -1263,6 +1258,7 @@ LUAG_FUNC( lane_new) STACK_CHECK(L, 1); STACK_CHECK( L2, 1 + nargs); + // TODO: launch thread earlier, sync with a std::latch to parallelize OS thread warmup and L2 preparation DEBUGSPEW_CODE( fprintf( stderr, INDENT_BEGIN "lane_new: launching thread\n" INDENT_END)); THREAD_CREATE(&lane->thread, lane_main, lane, priority); @@ -1306,7 +1302,7 @@ LUAG_FUNC(thread_gc) // We can read 'lane->status' without locks, but not wait for it // test Killed state first, as it doesn't need to enter the selfdestruct chain - if (lane->mstatus == ThreadStatus::Killed) + if (lane->mstatus == Lane::Killed) { // Make sure a kill has proceeded, before cleaning up the data structure. // @@ -1350,7 +1346,7 @@ LUAG_FUNC(thread_gc) } // Clean up after a (finished) thread - lane_cleanup(lane); + delete lane; // do this after lane cleanup in case the callback triggers an error if (have_gc_cb) @@ -1377,7 +1373,7 @@ static char const * thread_status_string(Lane* lane_) { enum e_status const st{ lane_->status }; // read just once (volatile) char const* str = - (lane_->mstatus == ThreadStatus::Killed) ? "killed" : // new to v3.3.0! + (lane_->mstatus == Lane::Killed) ? "killed" : // new to v3.3.0! (st == PENDING) ? "pending" : (st == RUNNING) ? "running" : // like in 'co.status()' (st == WAITING) ? "waiting" : @@ -1413,7 +1409,6 @@ LUAG_FUNC(thread_join) Lane* const lane{ lua_toLane(L, 1) }; lua_Number const wait_secs{ luaL_optnumber(L, 2, -1.0) }; lua_State* const L2{ lane->L }; - int ret; bool const done{ THREAD_ISNULL(lane->thread) || THREAD_WAIT(&lane->thread, wait_secs, &lane->done_signal, &lane->done_lock, &lane->status) }; if (!done || !L2) { @@ -1426,7 +1421,8 @@ LUAG_FUNC(thread_join) STACK_CHECK_START_REL(L, 0); // Thread is DONE/ERROR_ST/CANCELLED; all ours now - if (lane->mstatus == ThreadStatus::Killed) // OS thread was killed if thread_cancel was forced + int ret{ 0 }; + if (lane->mstatus == Lane::Killed) // OS thread was killed if thread_cancel was forced { // in that case, even if the thread was killed while DONE/ERROR_ST/CANCELLED, ignore regular return values STACK_GROW(L, 2); @@ -1536,7 +1532,7 @@ LUAG_FUNC(thread_index) switch (lane->status) { default: - if (lane->mstatus != ThreadStatus::Killed) + if (lane->mstatus != Lane::Killed) { // this is an internal error, we probably never get here lua_settop(L, 0); diff --git a/src/lanes_private.h b/src/lanes_private.h index 1530b9e..7c52876 100644 --- a/src/lanes_private.h +++ b/src/lanes_private.h @@ -4,40 +4,46 @@ #include "cancel.h" #include "universe.h" -enum class ThreadStatus -{ - Normal, // normal master side state - Killed // issued an OS kill -}; - // NOTE: values to be changed by either thread, during execution, without // locking, are marked "volatile" // -struct Lane +class Lane { + private: + + enum class ThreadStatus + { + Normal, // normal master side state + Killed // issued an OS kill + }; + + public: + + using enum ThreadStatus; + THREAD_T thread; // // M: sub-thread OS thread // S: not used - char const* debug_name; + char const* debug_name{ "" }; + Universe* const U; lua_State* L; - Universe* U; // // M: prepares the state, and reads results // S: while S is running, M must keep out of modifying the state - volatile enum e_status status; + volatile enum e_status status{ PENDING }; // // M: sets to PENDING (before launching) // S: updates -> RUNNING/WAITING -> DONE/ERROR_ST/CANCELLED - SIGNAL_T* volatile waiting_on; + SIGNAL_T* volatile waiting_on{ nullptr }; // // When status is WAITING, points on the linda's signal the thread waits on, else nullptr - volatile CancelRequest cancel_request; + volatile CancelRequest cancel_request{ CancelRequest::None }; // // M: sets to false, flags true for cancel request // S: reads to see if cancel is requested @@ -54,22 +60,31 @@ struct Lane // lane status changes to DONE/ERROR_ST/CANCELLED. #endif // THREADWAIT_METHOD == THREADWAIT_CONDVAR - volatile ThreadStatus mstatus; + volatile ThreadStatus mstatus{ Normal }; // // M: sets to Normal, if issued a kill changes to Killed // S: not used - Lane* volatile selfdestruct_next; + Lane* volatile selfdestruct_next{ nullptr }; // // M: sets to non-nullptr if facing lane handle '__gc' cycle but the lane // is still running // S: cleans up after itself if non-nullptr at lane exit #if HAVE_LANE_TRACKING() - Lane* volatile tracking_next; + Lane* volatile tracking_next{ nullptr }; #endif // HAVE_LANE_TRACKING() // // For tracking only + + static void* operator new(size_t size_, Universe* U_) noexcept { return U_->internal_allocator.alloc(size_); } + // 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(Lane)); } + // this one is for us, to make sure memory is freed by the correct allocator + static void operator delete(void* p_) { static_cast(p_)->U->internal_allocator.free(p_, sizeof(Lane)); } + + Lane(Universe* U_, lua_State* L_); + ~Lane(); }; // xxh64 of string "LANE_POINTER_REGKEY" generated at https://www.pelock.com/products/hash-calculator diff --git a/src/universe.h b/src/universe.h index 1079915..a2ad5f5 100644 --- a/src/universe.h +++ b/src/universe.h @@ -16,7 +16,7 @@ extern "C" { // forwards struct DeepPrelude; struct Keepers; -struct Lane; +class Lane; // ################################################################################################ -- cgit v1.2.3-55-g6feb