[td-225] fix invalid read for qhandle mgmt

src/client/src/tscLocal.c

@@ -406,7 +406,7 @@ int tscProcessLocalCmd(SSqlObj *pSql) {
     pSql->res.qhandle = 0x1;
     pSql->res.numOfRows = 0;
   } else if (pCmd->command == TSDB_SQL_RESET_CACHE) {
-    taosCacheEmpty(tscCacheHandle,false);
+    taosCacheEmpty(tscCacheHandle);
   } else if (pCmd->command == TSDB_SQL_SERV_VERSION) {
     tscProcessServerVer(pSql);
   } else if (pCmd->command == TSDB_SQL_CLI_VERSION) {

src/client/src/tscServer.c

@@ -1955,7 +1955,7 @@ int tscProcessUseDbRsp(SSqlObj *pSql) {
 }
 
 int tscProcessDropDbRsp(SSqlObj *UNUSED_PARAM(pSql)) {
-  taosCacheEmpty(tscCacheHandle, false);
+  taosCacheEmpty(tscCacheHandle);
   return 0;
 }
 
@@ -2001,7 +2001,7 @@ int tscProcessAlterTableMsgRsp(SSqlObj *pSql) {
 
     if (isSuperTable) {  // if it is a super table, reset whole query cache
       tscDebug("%p reset query cache since table:%s is stable", pSql, pTableMetaInfo->name);
-      taosCacheEmpty(tscCacheHandle, false);
+      taosCacheEmpty(tscCacheHandle);
     }
   }
 

src/query/src/qExecutor.c

@@ -6497,7 +6497,7 @@ void qSetQueryMgmtClosed(void* pQMgmt) {
   pQueryMgmt->closed = true;
   pthread_mutex_unlock(&pQueryMgmt->lock);
 
-  taosCacheEmpty(pQueryMgmt->qinfoPool, true);
+  taosCacheRefresh(pQueryMgmt->qinfoPool, freeqinfoFn);
 }
 
 void qCleanupQueryMgmt(void* pQMgmt) {

src/util/inc/tcache.h

@@ -24,14 +24,13 @@ extern "C" {
 #include "tref.h"
 #include "hash.h"
 
-typedef void (*__cache_freeres_fn_t)(void*);
+typedef void (*__cache_free_fn_t)(void*);
 
 typedef struct SCacheStatis {
   int64_t missCount;
   int64_t hitCount;
   int64_t totalAccess;
   int64_t refreshCount;
-  int32_t numOfCollision;
 } SCacheStatis;
 
 typedef struct SCacheDataNode {
@@ -70,7 +69,7 @@ typedef struct {
 //  void *          pTimer;
   SCacheStatis    statistics;
   SHashObj *      pHashTable;
-  __cache_freeres_fn_t freeFp;
+  __cache_free_fn_t freeFp;
   uint32_t        numOfElemsInTrash;  // number of element in trash
   uint8_t         deleting;           // set the deleting flag to stop refreshing ASAP.
   pthread_t       refreshWorker;
@@ -91,15 +90,7 @@ typedef struct {
  * @param fn                   free resource callback function
  * @return
  */
-SCacheObj *taosCacheInit(int32_t keyType, int64_t refreshTimeInSeconds, bool extendLifespan, __cache_freeres_fn_t fn, const char *cacheName);
+SCacheObj *taosCacheInit(int32_t keyType, int64_t refreshTimeInSeconds, bool extendLifespan, __cache_free_fn_t fn, const char *cacheName);
-
-/**
- * initialize the cache object and set the free object callback function
- * @param refreshTimeInSeconds
- * @param freeCb
- * @return
- */
-SCacheObj *taosCacheInitWithCb(int32_t keyType, int64_t refreshTimeInSeconds, bool extendLifespan, __cache_freeres_fn_t fn, const char *cacheName);
 
 /**
  * add data into cache
@@ -163,9 +154,8 @@ void taosCacheRelease(SCacheObj *pCacheObj, void **data, bool _remove);
 /**
  *  move all data node into trash, clear node in trash can if it is not referenced by any clients
  * @param handle
- * @param _remove  remove the data or not if refcount is greater than 0
  */
-void taosCacheEmpty(SCacheObj *pCacheObj, bool _remove);
+void taosCacheEmpty(SCacheObj *pCacheObj);
 
 /**
  * release all allocated memory and destroy the cache object.
@@ -180,6 +170,14 @@ void taosCacheEmpty(SCacheObj *pCacheObj, bool _remove);
  */
 void taosCacheCleanup(SCacheObj *pCacheObj);
 
+/**
+ *
+ * @param pCacheObj
+ * @param fp
+ * @return
+ */
+void taosCacheRefresh(SCacheObj *pCacheObj, __cache_free_fn_t fp);
+
 #ifdef __cplusplus
 }
 #endif

src/util/src/tcache.c

@@ -223,9 +223,9 @@ static void doCleanupDataCache(SCacheObj *pCacheObj);
  * refresh cache to remove data in both hash list and trash, if any nodes' refcount == 0, every pCacheObj->refreshTime
  * @param handle   Cache object handle
  */
-static void* taosCacheRefresh(void *handle);
+static void* taosCacheTimedRefresh(void *pCacheObj);
 
-SCacheObj *taosCacheInit(int32_t keyType, int64_t refreshTimeInSeconds, bool extendLifespan, __cache_freeres_fn_t fn, const char* cacheName) {
+SCacheObj *taosCacheInit(int32_t keyType, int64_t refreshTimeInSeconds, bool extendLifespan, __cache_free_fn_t fn, const char* cacheName) {
   if (refreshTimeInSeconds <= 0) {
     return NULL;
   }
@@ -261,7 +261,7 @@ SCacheObj *taosCacheInit(int32_t keyType, int64_t refreshTimeInSeconds, bool ext
   pthread_attr_init(&thattr);
   pthread_attr_setdetachstate(&thattr, PTHREAD_CREATE_JOINABLE);
 
-  pthread_create(&pCacheObj->refreshWorker, &thattr, taosCacheRefresh, pCacheObj);
+  pthread_create(&pCacheObj->refreshWorker, &thattr, taosCacheTimedRefresh, pCacheObj);
 
   pthread_attr_destroy(&thattr);
   return pCacheObj;
@@ -450,7 +450,7 @@ void taosCacheRelease(SCacheObj *pCacheObj, void **data, bool _remove) {
   }
 }
 
-void taosCacheEmpty(SCacheObj *pCacheObj, bool _remove) {
+void taosCacheEmpty(SCacheObj *pCacheObj) {
   SHashMutableIterator *pIter = taosHashCreateIter(pCacheObj->pHashTable);
   
   __cache_wr_lock(pCacheObj);
@@ -459,8 +459,8 @@ void taosCacheEmpty(SCacheObj *pCacheObj, bool _remove) {
       break;
     }
     
-    SCacheDataNode *pNode = *(SCacheDataNode **)taosHashIterGet(pIter);
+    SCacheDataNode *pNode = *(SCacheDataNode **) taosHashIterGet(pIter);
-    if (T_REF_VAL_GET(pNode) == 0 || _remove) {
+    if (T_REF_VAL_GET(pNode) == 0) {
       taosCacheReleaseNode(pCacheObj, pNode);
     } else {
       taosCacheMoveToTrash(pCacheObj, pNode);
@@ -469,7 +469,7 @@ void taosCacheEmpty(SCacheObj *pCacheObj, bool _remove) {
   __cache_unlock(pCacheObj);
   
   taosHashDestroyIter(pIter);
-  taosTrashCanEmpty(pCacheObj, _remove);
+  taosTrashCanEmpty(pCacheObj, false);
 }
 
 void taosCacheCleanup(SCacheObj *pCacheObj) {
@@ -623,8 +623,29 @@ void doCleanupDataCache(SCacheObj *pCacheObj) {
   free(pCacheObj);
 }
 
-void* taosCacheRefresh(void *handle) {
+static void doCacheRefresh(SCacheObj* pCacheObj, int64_t time, __cache_free_fn_t fp) {
-  SCacheObj *pCacheObj = (SCacheObj *)handle;
+  SHashMutableIterator *pIter = taosHashCreateIter(pCacheObj->pHashTable);
+
+  __cache_wr_lock(pCacheObj);
+  while (taosHashIterNext(pIter)) {
+    SCacheDataNode *pNode = *(SCacheDataNode **)taosHashIterGet(pIter);
+    if ((pNode->addedTime + pNode->lifespan * pNode->extendFactor) <= time && T_REF_VAL_GET(pNode) <= 0) {
+      taosCacheReleaseNode(pCacheObj, pNode);
+      continue;
+    }
+
+    if (fp) {
+      fp(pNode->data);
+    }
+  }
+
+  __cache_unlock(pCacheObj);
+
+  taosHashDestroyIter(pIter);
+}
+
+void* taosCacheTimedRefresh(void *handle) {
+  SCacheObj* pCacheObj = handle;
   if (pCacheObj == NULL) {
     uDebug("object is destroyed. no refresh retry");
     return NULL;
@@ -657,21 +678,8 @@ void* taosCacheRefresh(void *handle) {
 
     // refresh data in hash table
     if (elemInHash > 0) {
-      int64_t expiredTime = taosGetTimestampMs();
+      int64_t now = taosGetTimestampMs();
-
+      doCacheRefresh(pCacheObj, now, NULL);
-      SHashMutableIterator *pIter = taosHashCreateIter(pCacheObj->pHashTable);
-
-      __cache_wr_lock(pCacheObj);
-      while (taosHashIterNext(pIter)) {
-        SCacheDataNode *pNode = *(SCacheDataNode **)taosHashIterGet(pIter);
-        if ((pNode->addedTime + pNode->lifespan * pNode->extendFactor) <= expiredTime && T_REF_VAL_GET(pNode) <= 0) {
-          taosCacheReleaseNode(pCacheObj, pNode);
-        }
-      }
-
-      __cache_unlock(pCacheObj);
-
-      taosHashDestroyIter(pIter);
     }
 
     taosTrashCanEmpty(pCacheObj, false);
@@ -679,3 +687,12 @@ void* taosCacheRefresh(void *handle) {
 
   return NULL;
 }
+
+void taosCacheRefresh(SCacheObj *pCacheObj, __cache_free_fn_t fp) {
+  if (pCacheObj == NULL) {
+    return;
+  }
+
+  int64_t now = taosGetTimestampMs();
+  doCacheRefresh(pCacheObj, now, fp);
+}