fix: mutex and return value

source/util/src/tcompare.c

@@ -1208,12 +1208,13 @@ typedef struct UsingRegex {
   regex_t pRegex;
   int32_t lastUsedTime;
 } UsingRegex;
+typedef UsingRegex* HashRegexPtr;
 
 typedef struct RegexCache {
   SHashObj      *regexHash;
   void          *regexCacheTmr;
   void          *timer;
-  TdThreadMutex mutex;
+  SRWLatch      mutex;
   bool          exit;
 } RegexCache;
 static RegexCache sRegexCache;
@@ -1222,11 +1223,7 @@ static RegexCache sRegexCache;
 
 static void checkRegexCache(void* param, void* tmrId) {
   int32_t  code = 0;
-  code = taosThreadMutexLock(&sRegexCache.mutex);
+  taosRLockLatch(&sRegexCache.mutex);
-  if (code != 0) {
-    uError("[regex cache] checkRegexCache, Failed to lock mutex");
-    return;
-  }
   if(sRegexCache.exit) {
     goto _exit;
   }
@@ -1247,10 +1244,7 @@ static void checkRegexCache(void* param, void* tmrId) {
     }
   }
 _exit:
-  code = taosThreadMutexUnlock(&sRegexCache.mutex);
+  taosRUnLockLatch(&sRegexCache.mutex);
-  if(code != 0) {
-    uError("[regex cache] checkRegexCache, Failed to unlock mutex");
-  }
 }
 
 void regexCacheFree(void *ppUsingRegex) {
@@ -1273,10 +1267,7 @@ int32_t InitRegexCache() {
   }
 
   sRegexCache.exit = false;
-  if (taosThreadMutexInit(&sRegexCache.mutex, NULL) != 0) {
+  taosInitRWLatch(&sRegexCache.mutex);
-    uError("failed to init mutex");
-    return -1;
-  }
   sRegexCache.timer = taosTmrStart(checkRegexCache, REGEX_CACHE_CLEAR_TIME * 1000, NULL, sRegexCache.regexCacheTmr);
   if (sRegexCache.timer == NULL) {
     uError("failed to start regex cache timer");
@@ -1290,18 +1281,11 @@ void DestroyRegexCache(){
   int32_t code = 0;
   uInfo("[regex cache] destory regex cache");
   (void)taosTmrStopA(&sRegexCache.timer);
-  code = taosThreadMutexLock(&sRegexCache.mutex);
+  taosWLockLatch(&sRegexCache.mutex);
-  if (code != 0) {
-    uError("[regex cache] Failed to lock mutex");
-    return;
-  }
   sRegexCache.exit = true;
   taosHashCleanup(sRegexCache.regexHash);
   taosTmrCleanUp(sRegexCache.regexCacheTmr);
-  code  =  taosThreadMutexUnlock(&sRegexCache.mutex);
+  taosWUnLockLatch(&sRegexCache.mutex);
-  if (code != 0) {
-    uError("[regex cache] Failed to unlock mutex");
-  }
 }
 
 int32_t checkRegexPattern(const char *pPattern) {
@@ -1322,17 +1306,17 @@ int32_t checkRegexPattern(const char *pPattern) {
   return TSDB_CODE_SUCCESS;
 }
 
-UsingRegex **getRegComp(const char *pPattern) {
+int32_t getRegComp(const char *pPattern, HashRegexPtr **regexRet) {
-  UsingRegex **ppUsingRegex = (UsingRegex **)taosHashAcquire(sRegexCache.regexHash, pPattern, strlen(pPattern));
+  HashRegexPtr* ppUsingRegex = (HashRegexPtr*)taosHashAcquire(sRegexCache.regexHash, pPattern, strlen(pPattern));
   if (ppUsingRegex != NULL) {
     (*ppUsingRegex)->lastUsedTime = taosGetTimestampSec();
-    return ppUsingRegex;
+    *regexRet = ppUsingRegex;
+    return TSDB_CODE_SUCCESS;
   }
   UsingRegex *pUsingRegex = taosMemoryMalloc(sizeof(UsingRegex));
   if (pUsingRegex == NULL) {
-    terrno  = TSDB_CODE_OUT_OF_MEMORY;
     uError("Failed to Malloc when compile regex pattern %s.", pPattern);
-    return NULL;
+    return TSDB_CODE_OUT_OF_MEMORY;
   }
   int32_t cflags = REG_EXTENDED;
   int32_t ret = regcomp(&pUsingRegex->pRegex, pPattern, cflags);
@@ -1341,8 +1325,7 @@ UsingRegex **getRegComp(const char *pPattern) {
     (void)regerror(ret, &pUsingRegex->pRegex, msgbuf, tListLen(msgbuf));
     uError("Failed to compile regex pattern %s. reason %s", pPattern, msgbuf);
     taosMemoryFree(pUsingRegex);
-    terrno = TSDB_CODE_PAR_REGULAR_EXPRESSION_ERROR;
+    return TSDB_CODE_PAR_REGULAR_EXPRESSION_ERROR;
-    return NULL;
   }
 
   while (true) {
@@ -1350,8 +1333,7 @@ UsingRegex **getRegComp(const char *pPattern) {
     if (code != 0 && code != TSDB_CODE_DUP_KEY) {
       regexCacheFree(&pUsingRegex);
       uError("Failed to put regex pattern %s into cache, exception internal error.", pPattern);
-      terrno = code;
+      return code;
-      return NULL;
     } else if (code == TSDB_CODE_DUP_KEY) {
       terrno = 0;
     }
@@ -1367,7 +1349,8 @@ UsingRegex **getRegComp(const char *pPattern) {
     }
   }
   pUsingRegex->lastUsedTime = taosGetTimestampSec();
-  return ppUsingRegex;
+  *regexRet = ppUsingRegex;
+  return TSDB_CODE_SUCCESS;
 }
 
 void releaseRegComp(UsingRegex  **regex){
@@ -1397,14 +1380,15 @@ int32_t threadGetRegComp(regex_t **regex, const char *pPattern) {
     }
   }
 
-  UsingRegex **ppRegex = getRegComp(pPattern);
+  HashRegexPtr *ppRegex = NULL;
-  if (ppRegex == NULL) {
+  int32_t code = getRegComp(pPattern, &ppRegex);
-    return 1;
+  if (code != TSDB_CODE_SUCCESS) {
+    return code;
   }
   pOldPattern = taosStrdup(pPattern);
   if (NULL == pOldPattern) {
     uError("Failed to Malloc when compile regex pattern %s.", pPattern);
-    return TSDB_CODE_OUT_OF_MEMORY;
+    return terrno;
   }
   ppUsingRegex = ppRegex;
   pRegex  = &((*ppUsingRegex)->pRegex);

source/util/test/regexTest.cpp

@@ -12,7 +12,8 @@
 
 extern "C" {
   typedef struct UsingRegex UsingRegex;
-  UsingRegex** getRegComp(const char *pPattern);
+  typedef struct HashRegexPtr HashRegexPtr;
+  int32_t getRegComp(const char *pPattern, HashRegexPtr **regexRet);
   int32_t threadGetRegComp(regex_t **regex, const char *pPattern);
 }
 
@@ -66,8 +67,12 @@ TEST(testCase, regexCacheTest1) {
   auto start = std::chrono::high_resolution_clock::now();
 
   uint64_t t0 = taosGetTimestampUs();
-  for(int i = 0; i < times; i++) {
+  for (int i = 0; i < times; i++) {
-    UsingRegex** rex = getRegComp(s1);
+    HashRegexPtr* ret = NULL;
+    int32_t       code = getRegComp(s1, &ret);
+    if (code != 0) {
+      FAIL() << "Failed to compile regex pattern " << s1;
+    }
   }
   uint64_t t1 = taosGetTimestampUs();
 
@@ -97,8 +102,12 @@ TEST(testCase, regexCacheTest2) {
   auto start = std::chrono::high_resolution_clock::now();
 
   uint64_t t0 = taosGetTimestampUs();
-  for(int i = 0; i < times; i++) {
+  for (int i = 0; i < times; i++) {
-    UsingRegex** rex = getRegComp(s1);
+    HashRegexPtr* ret = NULL;
+    int32_t       code = getRegComp(s1, &ret);
+    if (code != 0) {
+      FAIL() << "Failed to compile regex pattern " << s1;
+    }
   }
   uint64_t t1 = taosGetTimestampUs();
 
@@ -129,9 +138,12 @@ TEST(testCase, regexCacheTest3) {
   auto start = std::chrono::high_resolution_clock::now();
 
   uint64_t t0 = taosGetTimestampUs();
-  for(int i = 0; i < times; i++) {
+  for (int i = 0; i < times; i++) {
-    UsingRegex** rex = getRegComp(s1);
+    HashRegexPtr* ret = NULL;
-    rex = getRegComp(s2);
+    int32_t       code = getRegComp(s1, &ret);
+    if (code != 0) {
+      FAIL() << "Failed to compile regex pattern " << s1;
+    }
   }
   uint64_t t1 = taosGetTimestampUs();
 
@@ -167,10 +179,18 @@ TEST(testCase, regexCacheTest4) {
   uint64_t t0 = taosGetTimestampUs();
   for (int i = 0; i < times; i++) {
     for (int j = 0; j < count; ++j) {
-      UsingRegex** rex = getRegComp(s1);
+      HashRegexPtr* ret = NULL;
+      int32_t       code = getRegComp(s1, &ret);
+      if (code != 0) {
+        FAIL() << "Failed to compile regex pattern " << s1;
+      }
     }
     for (int j = 0; j < count; ++j) {
-      UsingRegex** rex = getRegComp(s2);
+      HashRegexPtr* ret = NULL;
+      int32_t       code = getRegComp(s2, &ret);
+      if (code != 0) {
+        FAIL() << "Failed to compile regex pattern " << s2;
+      }
     }
   }
   uint64_t t1 = taosGetTimestampUs();
@@ -206,7 +226,8 @@ TEST(testCase, regexCacheTest4) {
   printf("'%s' and '%s' take place by turn(per %d count) regex(new) %d times:%" PRIu64 " us.\n", s1, s2, count, times, t3 - t2);
 }
 
-/*  It is not a good idea to test this case, because it will take a long time.
+//  It is not a good idea to test this case, because it will take a long time.
+/*
 TEST(testCase, regexCacheTest5) {
   int times = 10000;
   int count = 10000;
@@ -217,10 +238,18 @@ TEST(testCase, regexCacheTest5) {
   uint64_t t0 = taosGetTimestampUs();
   for (int i = 0; i < times; i++) {
     for (int j = 0; j < count; ++j) {
-      UsingRegex** rex = getRegComp(s1);
+      HashRegexPtr* ret = NULL;
+      int32_t       code = getRegComp(s1, &ret);
+      if (code != 0) {
+        FAIL() << "Failed to compile regex pattern " << s1;
+      }
     }
     for (int j = 0; j < count; ++j) {
-      UsingRegex** rex = getRegComp(s2);
+      HashRegexPtr* ret = NULL;
+      int32_t       code = getRegComp(s2, &ret);
+      if (code != 0) {
+        FAIL() << "Failed to compile regex pattern " << s2;
+      }
     }
   }
   uint64_t t1 = taosGetTimestampUs();
@@ -257,19 +286,27 @@ TEST(testCase, regexCacheTest5) {
 }
 
 TEST(testCase, regexCacheTest6) {
-  int times = 10000;
+  int  times = 10000;
-  int count = 1000;
+  int  count = 1000;
-  char    s1[] = "abc%*";
+  char s1[] = "abc%*";
-  char    s2[] = "abc";
+  char s2[] = "abc";
   auto start = std::chrono::high_resolution_clock::now();
 
   uint64_t t0 = taosGetTimestampUs();
   for (int i = 0; i < times; i++) {
     for (int j = 0; j < count; ++j) {
-      UsingRegex** rex = getRegComp(s1);
+      HashRegexPtr* ret = NULL;
+      int32_t       code = getRegComp(s1, &ret);
+      if (code != 0) {
+        FAIL() << "Failed to compile regex pattern " << s1;
+      }
     }
     for (int j = 0; j < count; ++j) {
-      UsingRegex** rex = getRegComp(s2);
+      HashRegexPtr* ret = NULL;
+      int32_t       code = getRegComp(s2, &ret);
+      if (code != 0) {
+        FAIL() << "Failed to compile regex pattern " << s2;
+      }
     }
   }
   uint64_t t1 = taosGetTimestampUs();