From 313454c426fdbd41cd41a4520b3029dca2d4ee0e Mon Sep 17 00:00:00 2001 From: Hayodea Hekol Date: Thu, 27 Nov 2025 22:26:50 -0400 Subject: [PATCH] OClCollMeshEngn: Add bridged delay in finalize() See the diff of the todo file within this commit for more details. In short, we do this to prevent the possibility of an in-flight async contin accessing metadata that we've already destroyed after finalize() has been called. --- .../openClCollatingAndMeshingEngine.cpp | 44 ++++++++++++++++++- .../openClCollatingAndMeshingEngine.h | 2 + todo | 13 +----- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.cpp b/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.cpp index 2714371..f32242d 100644 --- a/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.cpp +++ b/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.cpp @@ -6,8 +6,11 @@ #include #include #include +#include #include +#include #include +#include #include #include #include @@ -166,7 +169,46 @@ void OpenClCollatingAndMeshingEngine::finalize() // Complete any running kernels if (compactIsRunning) { compactKernelComplete(true); } - if (collateIsRunning) { collateKernelComplete(std::nullopt, std::nullopt, true); } + if (collateIsRunning) { + collateKernelComplete(std::nullopt, std::nullopt, true); + } + + { + /** EXPLANATION: + * Calculate the delay as the maximum of the configured delay and any + * future delays. The 0 is a placeholder for any delays that will be + * introduced in the future. When new delays are added, they should be + * included in the std::max() call (e.g., std::max( + * OCLCOLLMESH_ENGN_FINALIZE_DELAY_MS, futureDelay1, futureDelay2, 0)). + */ + int delayMs = std::max(OCLCOLLMESH_ENGN_FINALIZE_DELAY_MS, 0); + + auto& ioService = smoHooksPtr->ComponentThread_getSelf()->getIoService(); + AsynchronousBridge bridge(ioService); + boost::asio::deadline_timer timeoutTimer(ioService); + + /** EXPLANATION: + * We wait for delayMs milliseconds to ensure that any in-flight OpenCL + * kernel operations have definitely finished. OpenCL kernels cannot be + * cancelled once enqueued, so in-flight kernels may still be executing + * when finalize() is called. The delay ensures any running kernels + * complete and their callbacks execute before we destroy resources. + * This prevents use-after-free errors from resumed async continuations + * accessing destroyed state. + */ + timeoutTimer.expires_from_now( + boost::posix_time::milliseconds(delayMs)); + timeoutTimer.async_wait( + [&bridge](const boost::system::error_code& error) + { + (void)error; + + // Always signal complete, whether timeout expired or was cancelled + bridge.setAsyncOperationComplete(); + }); + + bridge.waitForAsyncOperationCompleteOrIoServiceStopped(); + } // Release OpenCL buffers via smo hooks if (smoHooksPtr && smoHooksPtr->ComputeManager_releaseUseHostPtrBuffer) diff --git a/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.h b/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.h index be96142..410f92d 100644 --- a/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.h +++ b/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.h @@ -21,6 +21,8 @@ #include #include +#define OCLCOLLMESH_ENGN_FINALIZE_DELAY_MS 1 + namespace smo { namespace stim_buff { diff --git a/todo b/todo index 1b8377c..239c1bf 100644 --- a/todo +++ b/todo @@ -38,18 +38,7 @@ PcloudStimProducer::stop=>start() sequence: - OpenClCollatingAndMeshingEngine::finalize(): -I'm also worried, though less so, about the OClCollMeshEngn: it's -a lot less likely to have an in-flight op run past the point where -the OClCollMeshEngn object has expired. -* But there's still a chance that a long-running OCl kernel could - cause an in-flight async contin to resume executing after its - OclCollMeshEngn has expired. -* We should do a bridged async wait for the std::max() of all - timeouts used by OClCollMeshEngn to pass before leaving - PcloudStimProducer::stop. - - Attaching and detaching StimBuffs from StimProducers: + Attaching and detaching StimBuffs from StimProducers: We've written code recently to attach and detact stimBuffs from a stimProducer. The code is quite nice, but there's this hanging omen over the fact that we put no thought into ensuring that