From dfcffc8d4c9fe7bbd4e06f59c133302b88e02c92 Mon Sep 17 00:00:00 2001 From: matlabbe Date: Sun, 27 Sep 2026 15:24:38 -0700 Subject: [PATCH] Added UScopeMutex::lockTry() support --- CMakeLists.txt | 2 +- package.xml | 2 +- utilite/include/rtabmap/utilite/UMutex.h | 104 +++++++++++++++++++-- utilite/test/test_umutex.cpp | 114 +++++++++++++++++++++++ 4 files changed, 214 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5783aba7..8bea0b1c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,7 +22,7 @@ SET(CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake_modules") ####################### SET(RTABMAP_MAJOR_VERSION 0) SET(RTABMAP_MINOR_VERSION 23) -SET(RTABMAP_PATCH_VERSION 12) +SET(RTABMAP_PATCH_VERSION 13) SET(RTABMAP_VERSION ${RTABMAP_MAJOR_VERSION}.${RTABMAP_MINOR_VERSION}.${RTABMAP_PATCH_VERSION}) diff --git a/package.xml b/package.xml index fb89be56..e11b6c20 100644 --- a/package.xml +++ b/package.xml @@ -1,7 +1,7 @@ rtabmap - 0.23.12 + 0.23.13 RTAB-Map's standalone library. RTAB-Map is a RGB-D SLAM approach with real-time constraints. Mathieu Labbe Mathieu Labbe diff --git a/utilite/include/rtabmap/utilite/UMutex.h b/utilite/include/rtabmap/utilite/UMutex.h index a10c2955..0490b460 100644 --- a/utilite/include/rtabmap/utilite/UMutex.h +++ b/utilite/include/rtabmap/utilite/UMutex.h @@ -159,28 +159,120 @@ public: * * @endcode * + * The lock can also be deferred, for example to only try locking it. The destructor + * then unlocks the mutex only if this object locked it: + * @code + * void callback() + * { + * UScopeMutex sm(m, false); // not locked yet + * if(sm.lockTry() == 0) + * { + * if(cond1) + * { + * return; // automatically unlock the mutex m + * } + * ... + * } + * // the mutex m is unlocked only if lockTry() succeeded + * } + * @endcode + * * @see UMutex */ class UScopeMutex { public: - UScopeMutex(const UMutex & mutex) : - mutex_(mutex) + /** + * @param mutex the mutex to lock. + * @param lockNow if true (default), the mutex is locked here. If false, it is not + * locked until lock() or lockTry() is called. + */ + UScopeMutex(const UMutex & mutex, bool lockNow = true) : + mutex_(mutex), + locked_(false) { - mutex_.lock(); + if(lockNow) + { + lock(); + } } // backward compatibility UScopeMutex(UMutex * mutex) : - mutex_(*mutex) + mutex_(*mutex), + locked_(false) { - mutex_.lock(); + lock(); } + /** + * Unlock the mutex, only if this object locked it. + */ ~UScopeMutex() { - mutex_.unlock(); + unlock(); } + + /** + * Lock the mutex, if this object doesn't hold it already. + * @return 0 on success, an error code otherwise. + */ + int lock() + { + if(locked_) + { + return 0; + } + int r = mutex_.lock(); + locked_ = r == 0; + return r; + } + +#if !defined(_WIN32) || (_WIN32_WINNT >= 0x0400) + /** + * Try locking the mutex, if this object doesn't hold it already. + * @return 0 if the mutex is held by this object, EBUSY (or another + * error code) otherwise. + */ + int lockTry() + { + if(locked_) + { + return 0; + } + int r = mutex_.lockTry(); + locked_ = r == 0; + return r; + } +#endif + + /** + * Unlock the mutex before this object goes out of scope, only if this object locked it. + * @return 0 on success (or if this object didn't hold the mutex), an error code otherwise. + */ + int unlock() + { + if(!locked_) + { + return 0; + } + locked_ = false; + return mutex_.unlock(); + } + + /** + * @return true if this object currently holds the mutex. + */ + bool isLocked() const + { + return locked_; + } + +private: + UScopeMutex(const UScopeMutex &); + void operator=(const UScopeMutex &); + private: const UMutex & mutex_; + bool locked_; }; #endif // UMUTEX_H diff --git a/utilite/test/test_umutex.cpp b/utilite/test/test_umutex.cpp index ca01a6eb..18f45dc8 100644 --- a/utilite/test/test_umutex.cpp +++ b/utilite/test/test_umutex.cpp @@ -149,6 +149,120 @@ TEST(UMutexTest, UScopeMutexWithPointer) t.join(); } +TEST(UMutexTest, UScopeMutexDeferredIsNotLocked) +{ + UMutex mutex; + { + UScopeMutex scopeMutex(mutex, false); + EXPECT_FALSE(scopeMutex.isLocked()); + + std::thread t([&mutex]() { + EXPECT_EQ(mutex.lockTry(), 0); // Not locked by the scope mutex + mutex.unlock(); + }); + t.join(); + } + // The destructor must not unlock a mutex the scope mutex didn't lock + mutex.lock(); + std::thread t([&mutex]() { + EXPECT_NE(mutex.lockTry(), 0); // Still locked by this thread + }); + t.join(); + mutex.unlock(); +} + +TEST(UMutexTest, UScopeMutexDeferredLock) +{ + UMutex mutex; + { + UScopeMutex scopeMutex(mutex, false); + EXPECT_EQ(scopeMutex.lock(), 0); + EXPECT_TRUE(scopeMutex.isLocked()); + + std::thread t([&mutex]() { + EXPECT_NE(mutex.lockTry(), 0); // Should fail + }); + t.join(); + } + std::thread t([&mutex]() { + EXPECT_EQ(mutex.lockTry(), 0); // Unlocked by the destructor + mutex.unlock(); + }); + t.join(); +} + +TEST(UMutexTest, UScopeMutexLockTrySucceeds) +{ + UMutex mutex; + { + UScopeMutex scopeMutex(mutex, false); + EXPECT_EQ(scopeMutex.lockTry(), 0); + EXPECT_TRUE(scopeMutex.isLocked()); + EXPECT_EQ(scopeMutex.lockTry(), 0); // Already held: not locked a second time + } + std::thread t([&mutex]() { + EXPECT_EQ(mutex.lockTry(), 0); // Unlocked once by the destructor, and free + mutex.unlock(); + }); + t.join(); +} + +TEST(UMutexTest, UScopeMutexLockTryFails) +{ + UMutex mutex; + std::atomic locked(false); + std::atomic release(false); + std::thread owner([&]() { + mutex.lock(); + locked = true; + while(!release) { std::this_thread::yield(); } + mutex.unlock(); + }); + while(!locked) { std::this_thread::yield(); } + + { + UScopeMutex scopeMutex(mutex, false); + EXPECT_NE(scopeMutex.lockTry(), 0); // Held by the other thread + EXPECT_FALSE(scopeMutex.isLocked()); + } + // The destructor didn't unlock the other thread's lock + std::thread t([&mutex]() { + EXPECT_NE(mutex.lockTry(), 0); + }); + t.join(); + + release = true; + owner.join(); + EXPECT_EQ(mutex.lockTry(), 0); + mutex.unlock(); +} + +TEST(UMutexTest, UScopeMutexEarlyUnlock) +{ + UMutex mutex; + { + UScopeMutex scopeMutex(mutex); + EXPECT_TRUE(scopeMutex.isLocked()); + EXPECT_EQ(scopeMutex.unlock(), 0); + EXPECT_FALSE(scopeMutex.isLocked()); + EXPECT_EQ(scopeMutex.unlock(), 0); // Nothing to unlock anymore + + std::thread t([&mutex]() { + EXPECT_EQ(mutex.lockTry(), 0); // Released before the end of the scope + mutex.unlock(); + }); + t.join(); + + mutex.lock(); // Locked by this thread, not by the scope mutex + } + // The destructor must not unlock it + std::thread t([&mutex]() { + EXPECT_NE(mutex.lockTry(), 0); + }); + t.join(); + mutex.unlock(); +} + TEST(UMutexTest, MultipleMutexes) { UMutex mutex1;