diff --git a/stimBuffApis/livoxGen1/ioUringAssemblyEngine.cpp b/stimBuffApis/livoxGen1/ioUringAssemblyEngine.cpp index 7912452..33a65bc 100644 --- a/stimBuffApis/livoxGen1/ioUringAssemblyEngine.cpp +++ b/stimBuffApis/livoxGen1/ioUringAssemblyEngine.cpp @@ -178,6 +178,36 @@ void IoUringAssemblyEngine::finalize() { bool wasAcceptingRequests = stop(); + { + auto& ioService = smoHooksPtr->ComponentThread_getSelf()->getIoService(); + AsynchronousBridge bridge(ioService); + boost::asio::deadline_timer timeoutTimer(ioService); + + /** EXPLANATION: + * We wait for IOURINGASSM_ENGN_FRAME_ASSEM_TIMEOUT_MS + 1 ms to ensure + * that any in-flight assembly operation has definitely finished. We add + * 1 ms because an operation could have just begun when we call stop(), + * and then it would timeout in IOURINGASSM_ENGN_FRAME_ASSEM_TIMEOUT_MS + * ms and potentially touch state that we have begun to destroy since it + * only just exited the delay. The extra 1 ms ensures that even if an + * operation started at the last possible moment, it will have completed + * or timed out before we proceed with cleanup. + */ + timeoutTimer.expires_from_now( + boost::posix_time::milliseconds( + IOURINGASSM_ENGN_FRAME_ASSEM_TIMEOUT_MS + 1)); + 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(); + } + if (eventfdFd >= 0) { io_uring_unregister_eventfd(&ring); @@ -446,10 +476,10 @@ public: context.get(), context, std::placeholders::_1, std::placeholders::_2)); - // Set up timeout timer for CONFIG_STIMBUFF_FRAME_PERIOD_MS/2 ms + // Set up timeout timer for IOURINGASSM_ENGN_FRAME_ASSEM_TIMEOUT_MS ms engine.stallTimer.expires_from_now( boost::posix_time::milliseconds( - CONFIG_STIMBUFF_FRAME_PERIOD_MS / 2)); + IOURINGASSM_ENGN_FRAME_ASSEM_TIMEOUT_MS)); engine.stallTimer.async_wait( std::bind(&AssembleFrameReq::assembleFrameReq2_1, context.get(), context, diff --git a/stimBuffApis/livoxGen1/ioUringAssemblyEngine.h b/stimBuffApis/livoxGen1/ioUringAssemblyEngine.h index 5d5563d..ae4fc4d 100644 --- a/stimBuffApis/livoxGen1/ioUringAssemblyEngine.h +++ b/stimBuffApis/livoxGen1/ioUringAssemblyEngine.h @@ -2,6 +2,7 @@ #define _LIVOX_GEN1_IOURING_ASSEMBLY_ENGINE_H #include +#include #include #include #include @@ -21,6 +22,9 @@ #include #include +#define IOURINGASSM_ENGN_FRAME_ASSEM_TIMEOUT_MS \ + (CONFIG_STIMBUFF_FRAME_PERIOD_MS / 2) + namespace smo { namespace stim_buff { diff --git a/todo b/todo index 7e85414..1b8377c 100644 --- a/todo +++ b/todo @@ -37,23 +37,6 @@ timeslice deferrals. PcloudStimProducer::stop=>start() sequence: - IoUringAssemblyEngine::finalize(): -I'm worried that calling PcloudStimProducer::stop() will leave -in-flight sequences running which will remain alive even after -the PcloudStimProducer object itself has been destroyed. This may -be possible for IoUringAssmEngn because it has a running timer -which may well just time out. -* There's no reason to think that an in-flight IoUringAssmEngn - assembly operation won't actually run until it times out. In - fact, that's the standard case if you configure - nDgramsPerFrame to be large enough. -* This means that when we call IoUringAssmEngn::finalize(), an - in-flight assembly could be going on, which isn't receiving - any CQE notifications on the eventFd. Thus, that in-flight - assembly op could plausibly timeout and resume execution - after IoUringAssemEngn::finalize has completed. -* We ought to do a bridged async timeout for the std::max() - of all timeouts used by IoUringAssmEngn. OpenClCollatingAndMeshingEngine::finalize(): I'm also worried, though less so, about the OClCollMeshEngn: it's