i/rtabmap
1
0
Fork 0
mirror of https://github.com/introlab/rtabmap.git synced 2026-10-04 00:57:46 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(DBDriver): resolve lock-order inversion between dbSafeAccess and trashes mutexes (#1775)

Browse Source 
* fix(DBDriver): resolve lock-order inversion between dbSafeAccess and trashes mutexes

emptyTrashes() acquired _dbSafeAccessMutex while holding _trashesMutex
(M1->M0), whereas the load() path acquires _dbSafeAccessMutex and then,
inside loadQuery()->getLastWordId(), acquires _trashesMutex (M0->M1).
This opposite nesting forms a lock-order cycle that ThreadSanitizer flags
as a potential deadlock.

Acquire _dbSafeAccessMutex only after releasing _trashesMutex, matching
the sequential 'look in trash, then database' pattern used by every other
DBDriver accessor (getLastWordId, getLastMapId, getInvertedIndexNi, ...).

Refs: #1765

* Fxing the actual deadlock

* Added doc

---------

Co-authored-by: matlabbe <[email protected]>
This commit is contained in:
Golitsin Vyacheslav
2026-09-27 22:43:06 -07:00
committed by GitHub
co-authored by matlabbe
parent 5c9cfa98fe
commit c59e0d7c35
2 changed files with 11 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
corelib/include/rtabmap/core/DBDriver.h
+9
View File
@@ -331,6 +331,12 @@ protected:
/**
* @name Backend implementation (subclass responsibility)
* @brief Pure virtual SQL/backend hooks invoked by public wrappers above.
*
* These are called with \c _dbSafeAccessMutex locked. Implementations must not
* call public methods that look in the trash (they lock \c _trashesMutex), as it
* would invert the lock order used by emptyTrashes() and could deadlock. Call the
* corresponding \c *Query() method directly instead (e.g., getLastIdQuery("Word", id)
* instead of getLastWordId(id)).
* @{*/
virtual bool connectDatabaseQuery(const std::string & url, bool overwritten = false, bool readOnly = false) = 0;
virtual void disconnectDatabaseQuery(bool save = true, const std::string & outputUrl = "") = 0;
@@ -457,6 +463,9 @@ private:
UMutex _transactionMutex;
std::map<int, Signature *> _trashSignatures;//<id, Signature*>
std::map<int, VisualWord *> _trashVisualWords; //<id, VisualWord*>
// Lock order: _trashesMutex -> _dbSafeAccessMutex -> _transactionMutex.
// emptyTrashes() locks _dbSafeAccessMutex before releasing _trashesMutex, so that
// an item not found in the trash is guaranteed to be readable from the database.
UMutex _trashesMutex;
UMutex _dbSafeAccessMutex;
USemaphore _addSem;
corelib/src/DBDriverSqlite3.cpp
+2 -2
View File
@@ -3624,8 +3624,8 @@ void DBDriverSqlite3::loadQuery(VWDictionary & dictionary, bool lastStateOnly, b
rc = sqlite3_finalize(ppStmt);
UASSERT_MSG(rc == SQLITE_OK, uFormat("DB error (%s): %s", _version.c_str(), sqlite3_errmsg(_ppDb)).c_str());
// Get Last word id
getLastWordId(id);
// Get Last word id (query directly: _dbSafeAccessMutex is already locked by DBDriver::load())
getLastIdQuery("Word", id);
dictionary.setLastWordId(id);
if(!idsOnly && uStrNumCmp(_version, "0.23.0") >= 0) {