From 07609c6d6c1ee378296f2602b02ae6266944326b Mon Sep 17 00:00:00 2001 From: Hayodea Hekol Date: Tue, 30 Sep 2025 20:48:45 -0400 Subject: [PATCH] SenseApiLib:Add isBeingDestroyed atomic flag for getter bailout Since we have no choice but to access the sh_ptr for a lib before we can get its Qutex, we use this flag to ensure that we can know whether the SenseApiLib data structure and its Qutex are still valid when we enter -- i.e, we ensure that the SenseApiLib object wasn't destroyed under our feet. --- smocore/deviceManager/deviceManager.cpp | 49 +++++++++---------------- smocore/include/senseApis/senseApiLib.h | 4 +- smocore/senseApis/senseApiManager.cpp | 9 +++++ 3 files changed, 30 insertions(+), 32 deletions(-) diff --git a/smocore/deviceManager/deviceManager.cpp b/smocore/deviceManager/deviceManager.cpp index 35111ee..4ab4d36 100644 --- a/smocore/deviceManager/deviceManager.cpp +++ b/smocore/deviceManager/deviceManager.cpp @@ -324,10 +324,11 @@ public: const std::shared_ptr& spec, const std::shared_ptr &caller, Callback cb, + std::shared_ptr &senseApiLib, std::vector> requiredLocks) : SerializedAsynchronousContinuation( caller, cb, requiredLocks), - spec(spec) + spec(spec), senseApiLib(senseApiLib) {} public: @@ -344,26 +345,18 @@ public: return; } - /** FIXME: - * We should acquire a spinlock here to ensure that the device isn't - * added in the interim while the async op executes. - */ - auto libOpt = sense_api::SenseApiManager::getInstance() - .getSenseApiLibByApiName(spec->api); - - if (!libOpt) + if (senseApiLib->isBeingDestroyed.load()) { - std::cerr << std::string(__func__) + ": No library found for API '" - << spec->api << "'" << std::endl; + std::cerr << std::string(__func__) + ": Library is being destroyed" + << " for API '" << spec->api << "'. Bailing out." << std::endl; callOriginalCb(false, spec); return; } - auto& lib = *libOpt.value(); - if (!lib.senseApiDesc.sal_mgmt_libOps.attachDeviceReq) + if (!senseApiLib->senseApiDesc.sal_mgmt_libOps.attachDeviceReq) { std::cerr << std::string(__func__) + ": attachDeviceReq() is NULL " - "for library '" << lib.libraryPath << "'" << std::endl; + "for library '" << senseApiLib->libraryPath << "'" << std::endl; callOriginalCb(false, spec); return; } @@ -391,7 +384,7 @@ public: << spec->deviceIdentifier << " to body thread" << "\n"; } - lib.senseApiDesc.sal_mgmt_libOps.attachDeviceReq( + senseApiLib->senseApiDesc.sal_mgmt_libOps.attachDeviceReq( spec, threadForAttachment, {context, std::bind( &AttachSenseDeviceReq::attachSenseDeviceReq2, @@ -421,32 +414,25 @@ public: return; } - /** FIXME: - * We should acquire a spinlock here to ensure that the device isn't - * removed in the interim while the async op executes. - */ - auto libOpt = sense_api::SenseApiManager::getInstance() - .getSenseApiLibByApiName(spec->api); - - if (!libOpt) + if (senseApiLib->isBeingDestroyed.load()) { - std::cerr << std::string(__func__) + ": No library found for API '" - << spec->api << "'" << std::endl; + std::cerr << std::string(__func__) + ": Library is being destroyed" + << " for API '" << spec->api << "'. Bailing out." << std::endl; callOriginalCb(false, spec); return; } - auto& lib = *libOpt.value(); - if (!lib.senseApiDesc.sal_mgmt_libOps.detachDeviceReq) + + if (!senseApiLib->senseApiDesc.sal_mgmt_libOps.detachDeviceReq) { std::cerr << std::string(__func__) + ": detachDeviceReq() is NULL " - "for library '" << lib.libraryPath << "'" << std::endl; + "for library '" << senseApiLib->libraryPath << "'" << std::endl; callOriginalCb(false, spec); return; } sense_api::SenseApiManager::getInstance().qutex.release(); - lib.senseApiDesc.sal_mgmt_libOps.detachDeviceReq( + senseApiLib->senseApiDesc.sal_mgmt_libOps.detachDeviceReq( spec, {context, std::bind( &DetachSenseDeviceReq::detachSenseDeviceReq2, @@ -465,6 +451,7 @@ public: public: std::shared_ptr spec; + std::shared_ptr senseApiLib; }; void DeviceManager::attachSenseDeviceReq( @@ -489,7 +476,7 @@ void DeviceManager::attachSenseDeviceReq( auto& lib = *libOpt.value(); auto request = std::make_shared( - spec, caller, cb, + spec, caller, cb, libOpt.value(), LockSet::Set{ std::ref(sense_api::SenseApiManager::getInstance().qutex), std::ref(lib.qutex) @@ -524,7 +511,7 @@ void DeviceManager::detachSenseDeviceReq( auto& lib = *libOpt.value(); auto request = std::make_shared( - spec, caller, cb, + spec, caller, cb, libOpt.value(), LockSet::Set{ std::ref(sense_api::SenseApiManager::getInstance().qutex), std::ref(lib.qutex) diff --git a/smocore/include/senseApis/senseApiLib.h b/smocore/include/senseApis/senseApiLib.h index 0ff1a71..f59831b 100644 --- a/smocore/include/senseApis/senseApiLib.h +++ b/smocore/include/senseApis/senseApiLib.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -30,7 +31,7 @@ public: SenseApiLib( const std::string& path, void *_dlopen_handle, SMO_GET_SENSE_API_DESC_FN_TYPEDEF *descFn) - : libraryPath(path), qutex("SenseApiLib-" + path), + : libraryPath(path), qutex("SenseApiLib-" + path), isBeingDestroyed(false), dlopen_handle(_dlopen_handle, DlCloser()), SMO_GET_SENSE_API_DESC_FN_NAME(descFn) {} @@ -50,6 +51,7 @@ public: public: std::string libraryPath; Qutex qutex; + std::atomic isBeingDestroyed; std::unique_ptr dlopen_handle; /* UNIMPLEMENTED: API-specific cmdline options. These affect this specific * sense api lib's behaviour globally. diff --git a/smocore/senseApis/senseApiManager.cpp b/smocore/senseApis/senseApiManager.cpp index cbe637b..2b0adf2 100644 --- a/smocore/senseApis/senseApiManager.cpp +++ b/smocore/senseApis/senseApiManager.cpp @@ -228,6 +228,10 @@ std::string SenseApiManager::stringifyLibs() const void SenseApiManager::initializeSenseApiLib(SenseApiLib& lib) { + /** FIXME: + * When we eventually make this method async, this method should acquire + * the SenseApiManager's main CRUD qutex. + */ if (!lib.senseApiDesc.sal_mgmt_libOps.initializeInd) { throw std::runtime_error( @@ -239,6 +243,11 @@ void SenseApiManager::initializeSenseApiLib(SenseApiLib& lib) void SenseApiManager::finalizeSenseApiLib(SenseApiLib& lib) { + /** FIXME: + * When we eventually make this method async, this flag should only be set + * after acquiring the SenseApiManager's main CRUD qutex. + */ + lib.isBeingDestroyed.store(true); if (!lib.senseApiDesc.sal_mgmt_libOps.finalizeInd) { throw std::runtime_error(