IoUringAssmEngn: Add ~16ms bridged delay in finalize()
See the diff of the todo file within this patch for more details. This is to eliminate the possibility of having an in-flight async contin access metadata that we destroyed in finalize().
This commit is contained in:
@@ -178,6 +178,36 @@ void IoUringAssemblyEngine::finalize()
|
|||||||
{
|
{
|
||||||
bool wasAcceptingRequests = stop();
|
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)
|
if (eventfdFd >= 0)
|
||||||
{
|
{
|
||||||
io_uring_unregister_eventfd(&ring);
|
io_uring_unregister_eventfd(&ring);
|
||||||
@@ -446,10 +476,10 @@ public:
|
|||||||
context.get(), context,
|
context.get(), context,
|
||||||
std::placeholders::_1, std::placeholders::_2));
|
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(
|
engine.stallTimer.expires_from_now(
|
||||||
boost::posix_time::milliseconds(
|
boost::posix_time::milliseconds(
|
||||||
CONFIG_STIMBUFF_FRAME_PERIOD_MS / 2));
|
IOURINGASSM_ENGN_FRAME_ASSEM_TIMEOUT_MS));
|
||||||
engine.stallTimer.async_wait(
|
engine.stallTimer.async_wait(
|
||||||
std::bind(&AssembleFrameReq::assembleFrameReq2_1,
|
std::bind(&AssembleFrameReq::assembleFrameReq2_1,
|
||||||
context.get(), context,
|
context.get(), context,
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
#define _LIVOX_GEN1_IOURING_ASSEMBLY_ENGINE_H
|
#define _LIVOX_GEN1_IOURING_ASSEMBLY_ENGINE_H
|
||||||
|
|
||||||
#include <boostAsioLinkageFix.h>
|
#include <boostAsioLinkageFix.h>
|
||||||
|
#include <config.h>
|
||||||
#include <cstdint>
|
#include <cstdint>
|
||||||
#include <cstddef>
|
#include <cstddef>
|
||||||
#include <memory>
|
#include <memory>
|
||||||
@@ -21,6 +22,9 @@
|
|||||||
#include <spinLock.h>
|
#include <spinLock.h>
|
||||||
#include <user/frameAssemblyDesc.h>
|
#include <user/frameAssemblyDesc.h>
|
||||||
|
|
||||||
|
#define IOURINGASSM_ENGN_FRAME_ASSEM_TIMEOUT_MS \
|
||||||
|
(CONFIG_STIMBUFF_FRAME_PERIOD_MS / 2)
|
||||||
|
|
||||||
namespace smo {
|
namespace smo {
|
||||||
namespace stim_buff {
|
namespace stim_buff {
|
||||||
|
|
||||||
|
|||||||
@@ -37,23 +37,6 @@
|
|||||||
timeslice deferrals.
|
timeslice deferrals.
|
||||||
|
|
||||||
PcloudStimProducer::stop=>start() sequence:
|
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():
|
OpenClCollatingAndMeshingEngine::finalize():
|
||||||
I'm also worried, though less so, about the OClCollMeshEngn: it's
|
I'm also worried, though less so, about the OClCollMeshEngn: it's
|
||||||
|
|||||||
Reference in New Issue
Block a user