1. rename recipes to patterns. 2. some minor fix

fhector · Oct 19, 2022 · 8fbe952 · 8fbe952
1 parent 6333b64
commit 8fbe952
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 27 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -69,8 +69,10 @@ set(REDIS_PLUS_PLUS_SOURCES
         "${REDIS_PLUS_PLUS_SOURCE_DIR}/transaction.cpp"
 )
 
+set(PATTERNS_DIR "${REDIS_PLUS_PLUS_SOURCE_DIR}/patterns")
+
 list(APPEND REDIS_PLUS_PLUS_SOURCES
-        "${REDIS_PLUS_PLUS_SOURCE_DIR}/recipes/redlock.cpp")
+        "${PATTERNS_DIR}/redlock.cpp")
 
 if(REDIS_PLUS_PLUS_BUILD_ASYNC)
     list(APPEND REDIS_PLUS_PLUS_SOURCES
@@ -176,6 +178,7 @@ if(REDIS_PLUS_PLUS_BUILD_STATIC)
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${REDIS_PLUS_PLUS_SOURCE_DIR}>
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${TLS_SUB_DIR}>
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${CXX_UTILS_DIR}>
+            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${PATTERNS_DIR}>
             $<INSTALL_INTERFACE:include>)
 
     if(hiredis_FOUND)
@@ -228,6 +231,7 @@ if(REDIS_PLUS_PLUS_BUILD_SHARED)
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${REDIS_PLUS_PLUS_SOURCE_DIR}>
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${TLS_SUB_DIR}>
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${CXX_UTILS_DIR}>
+            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${PATTERNS_DIR}>
             $<INSTALL_INTERFACE:include>)
 
     if(hiredis_FOUND)
@@ -305,8 +309,8 @@ endif()
 
 install(FILES ${HEADERS} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/${HEADER_PATH})
 
-file(GLOB RECIPES_HEADERS "${REDIS_PLUS_PLUS_SOURCE_DIR}/recipes/*.h*")
-install(FILES ${RECIPES_HEADERS} DESTINATION ${CMAKE_INSTALL_PREFIX}/include/${HEADER_PATH}/recipes)
+file(GLOB PATTERNS_HEADERS "${PATTERNS_DIR}/*.h*")
+install(FILES ${PATTERNS_HEADERS} DESTINATION ${CMAKE_INSTALL_PREFIX}/include/${HEADER_PATH}/patterns)
 
 include(CMakePackageConfigHelpers)
 

diff --git a/src/sw/redis++/recipes/redlock.cpp → src/sw/redis++/patterns/redlock.cpp b/src/sw/redis++/recipes/redlock.cpp → src/sw/redis++/patterns/redlock.cpp
@@ -348,8 +348,6 @@ void RedLockMutexVessel::unlock(const LockInfo& lock_info)
 void RedMutexImpl::lock() {
     std::lock_guard<std::mutex> lock(_mtx);
 
-    _sanity_check();
-
     if (_locked()) {
         throw Error("RedMutex is not reentrant");
     }
@@ -366,23 +364,23 @@ void RedMutexImpl::lock() {
 void RedMutexImpl::unlock() {
     std::lock_guard<std::mutex> lock(_mtx);
 
-    _sanity_check();
-
     if (!_locked()) {
         throw Error("RedMutex is not locked");
     }
 
-    // TODO: what if unlock fail?
-    _unlock(_lock_id);
+    try {
+        _unlock(_lock_id);
+    } catch (...) {
+        _reset();
+        throw;
+    }
 
-    _lock_id.clear();
+    _reset();
 }
 
 bool RedMutexImpl::try_lock() {
     std::lock_guard<std::mutex> lock(_mtx);
 
-    _sanity_check();
-
     if (_locked()) {
         throw Error("RedMutex is not reentrant");
     }
@@ -399,8 +397,6 @@ bool RedMutexImpl::try_lock() {
 bool RedMutexImpl::extend_lock() {
     std::lock_guard<std::mutex> lock(_mtx);
 
-    _sanity_check();
-
     if (!_locked()) {
         throw Error("cannot extend an unlocked RedMutex");
     }
@@ -418,7 +414,7 @@ bool RedMutexImpl::extend_lock() {
             }
         }
 
-        _valid = false;
+        _reset();
 
         return false;
     }
@@ -429,8 +425,6 @@ bool RedMutexImpl::extend_lock() {
 bool RedMutexImpl::locked() {
     std::lock_guard<std::mutex> lock(_mtx);
 
-    _sanity_check();
-
     return _locked();
 }
 

diff --git a/src/sw/redis++/recipes/redlock.h → src/sw/redis++/patterns/redlock.h b/src/sw/redis++/recipes/redlock.h → src/sw/redis++/patterns/redlock.h
@@ -389,10 +389,8 @@ class RedMutexImpl : public std::enable_shared_from_this<RedMutexImpl> {
 
     virtual bool _try_lock(const std::string &lock_id, const std::chrono::milliseconds &ttl) = 0;
 
-    void _sanity_check() const {
-        if (!_valid) {
-            throw Error("RedMutex is invalid");
-        }
+    void _reset() {
+        _lock_id.clear();
     }
 
     bool _locked() const {
@@ -405,8 +403,6 @@ class RedMutexImpl : public std::enable_shared_from_this<RedMutexImpl> {
 
     std::string _lock_id;
 
-    bool _valid = true;
-
     std::shared_ptr<LockWatcher> _watcher;
 
     std::function<void (std::exception_ptr)> _auto_extend_err_callback;

diff --git a/src/sw/redis++/redis++.h b/src/sw/redis++/redis++.h
@@ -21,6 +21,5 @@
 #include "redis_cluster.h"
 #include "queued_redis.h"
 #include "sentinel.h"
-#include "recipes/redlock.h"
 
 #endif // end SEWENEW_REDISPLUSPLUS_REDISPLUSPLUS_H
diff --git a/test/src/sw/redis++/redlock_test.h b/test/src/sw/redis++/redlock_test.h
@@ -18,6 +18,7 @@
 #define REDISPLUSPLUS_TEST_REDLOCK_TEST_H
 
 #include <sw/redis++/redis++.h>
+#include <sw/redis++/patterns/redlock.h>
 #include <memory>
 
 namespace sw {

diff --git a/test/src/sw/redis++/redlock_test.hpp b/test/src/sw/redis++/redlock_test.hpp
@@ -148,7 +148,7 @@ void RedLockTest<Redis>::run() {
     {
         std::queue<RedLockMutex*> mutex_list;
         for (int i=0; i<n; i++) {
-            mutex_list.push(new RedLockMutex(std::ref(_redis), RedLockUtils::lock_id()));
+            mutex_list.push(new RedLockMutex(_redis, RedLockUtils::lock_id()));
             const std::chrono::time_point<std::chrono::system_clock> tp = std::chrono::system_clock::now() + multi_lock_ttl;
             if (mutex_list.back()->try_lock(random_string, tp) < std::chrono::milliseconds(0)) {
                 std::cout << "Num locks = " << i << std::endl;;
@@ -171,7 +171,7 @@ void RedLockTest<Redis>::run() {
         std::queue<RedMutexTx*> mutex_list;
         for (int i=0; i<n; i++) {
             const std::chrono::time_point<std::chrono::system_clock> tp = std::chrono::system_clock::now() + multi_lock_ttl;
-            mutex_list.push(new RedMutexTx(std::ref(_redis), RedLockUtils::lock_id()));
+            mutex_list.push(new RedMutexTx(_redis, RedLockUtils::lock_id()));
             if (mutex_list.back()->try_lock(random_string, tp) < std::chrono::milliseconds(0)) {
                 REDIS_ASSERT(0, "unable to obtain a lock");
             }
@@ -284,7 +284,7 @@ void RedLockTest<Redis>::run() {
     // Locking should fail, on duplicate instances.
     {
         // We now use the same instance twice, which is expected to fail on locking.
-        RedLockMutexVessel redlock_2_identical_instances({std::ref(_redis), std::ref(_redis)});
+        RedLockMutexVessel redlock_2_identical_instances({_redis, _redis});
         const auto lock_info = redlock_2_identical_instances.lock(resource, random_string, ttl);
         if (lock_info.locked) {
             redlock_2_identical_instances.unlock(lock_info);