From 40fc4b6cf38c0373de43be0f68638fd858c1a562 Mon Sep 17 00:00:00 2001 From: yihaoDeng Date: Sun, 2 Jan 2022 17:50:40 +0800 Subject: [PATCH] fix mem leak and invalid read/write found by valgrind --- source/libs/index/inc/index_tfile.h | 1 + source/libs/index/src/index.c | 3 ++- source/libs/index/src/index_cache.c | 1 + source/libs/index/src/index_fst.c | 2 ++ source/libs/index/src/index_tfile.c | 9 ++++++--- source/libs/index/test/indexTests.cc | 29 +++++++++++++++++++++++++--- 6 files changed, 38 insertions(+), 7 deletions(-) diff --git a/source/libs/index/inc/index_tfile.h b/source/libs/index/inc/index_tfile.h index 675203fa1a..4618a39197 100644 --- a/source/libs/index/inc/index_tfile.h +++ b/source/libs/index/inc/index_tfile.h @@ -117,6 +117,7 @@ int tfileWriterFinish(TFileWriter* tw); // IndexTFile* indexTFileCreate(const char* path); +void indexTFileDestroy(IndexTFile* tfile); int indexTFilePut(void* tfile, SIndexTerm* term, uint64_t uid); int indexTFileSearch(void* tfile, SIndexTermQuery* query, SArray* result); diff --git a/source/libs/index/src/index.c b/source/libs/index/src/index.c index 11a2aa5e5f..9c7320b301 100644 --- a/source/libs/index/src/index.c +++ b/source/libs/index/src/index.c @@ -103,6 +103,7 @@ void indexClose(SIndex* sIdx) { } taosHashCleanup(sIdx->colObj); pthread_mutex_destroy(&sIdx->mtx); + indexTFileDestroy(sIdx->tindex); #endif free(sIdx->path); free(sIdx); @@ -479,7 +480,7 @@ void iterateValueDestroy(IterateValue* value, bool destroy) { } else { if (value->val != NULL) { taosArrayClear(value->val); } } - // free(value->colVal); + free(value->colVal); value->colVal = NULL; } static int indexGenTFile(SIndex* sIdx, IndexCache* cache, SArray* batch) { diff --git a/source/libs/index/src/index_cache.c b/source/libs/index/src/index_cache.c index 0f00d9d4af..b4c533e998 100644 --- a/source/libs/index/src/index_cache.c +++ b/source/libs/index/src/index_cache.c @@ -150,6 +150,7 @@ Iterate* indexCacheIteratorCreate(IndexCache* cache) { MemTable* tbl = cache->imm; iiter->val.val = taosArrayInit(1, sizeof(uint64_t)); + iiter->val.colVal = NULL; iiter->iter = tbl != NULL ? tSkipListCreateIter(tbl->mem) : NULL; iiter->next = indexCacheIteratorNext; iiter->getValue = indexCacheIteratorGetValue; diff --git a/source/libs/index/src/index_fst.c b/source/libs/index/src/index_fst.c index 088c7369d5..bfaeeaaa33 100644 --- a/source/libs/index/src/index_fst.c +++ b/source/libs/index/src/index_fst.c @@ -1062,6 +1062,7 @@ Output fstEmptyFinalOutput(Fst* fst, bool* null) { } else { *null = true; } + fstNodeDestroy(node); return res; } @@ -1286,6 +1287,7 @@ StreamWithStateResult* streamWithStateNextWith(StreamWithState* sws, StreamCallb StreamWithStateResult* result = swsResultCreate(&slice, fOutput, tState); free(buf); fstSliceDestroy(&slice); + taosArrayDestroy(nodes); return result; } free(buf); diff --git a/source/libs/index/src/index_tfile.c b/source/libs/index/src/index_tfile.c index fe54b5ebb3..95c713fb0a 100644 --- a/source/libs/index/src/index_tfile.c +++ b/source/libs/index/src/index_tfile.c @@ -360,7 +360,7 @@ IndexTFile* indexTFileCreate(const char* path) { tfile->cache = tfileCacheCreate(path); return tfile; } -void IndexTFileDestroy(IndexTFile* tfile) { +void indexTFileDestroy(IndexTFile* tfile) { tfileCacheDestroy(tfile->cache); free(tfile); } @@ -415,7 +415,7 @@ static bool tfileIteratorNext(Iterate* iiter) { static IterateValue* tifileIterateGetValue(Iterate* iter) { return &iter->val; } static TFileFstIter* tfileFstIteratorCreate(TFileReader* reader) { - TFileFstIter* tIter = calloc(1, sizeof(Iterate)); + TFileFstIter* tIter = calloc(1, sizeof(TFileFstIter)); if (tIter == NULL) { return NULL; } tIter->ctx = automCtxCreate(NULL, AUTOMATION_ALWAYS); @@ -437,6 +437,7 @@ Iterate* tfileIteratorCreate(TFileReader* reader) { iter->next = tfileIteratorNext; iter->getValue = tifileIterateGetValue; iter->val.val = taosArrayInit(1, sizeof(uint64_t)); + iter->val.colVal = NULL; return iter; } void tfileIteratorDestroy(Iterate* iter) { @@ -449,6 +450,7 @@ void tfileIteratorDestroy(Iterate* iter) { streamWithStateDestroy(tIter->st); fstStreamBuilderDestroy(tIter->fb); automCtxDestroy(tIter->ctx); + free(tIter); free(iter); } @@ -482,7 +484,7 @@ static int tfileValueCompare(const void* a, const void* b, const void* param) { TFileValue* tfileValueCreate(char* val) { TFileValue* tf = calloc(1, sizeof(TFileValue)); if (tf == NULL) { return NULL; } - tf->colVal = val; + tf->colVal = tstrdup(val); tf->tableId = taosArrayInit(32, sizeof(uint64_t)); return tf; } @@ -493,6 +495,7 @@ int tfileValuePush(TFileValue* tf, uint64_t val) { } void tfileValueDestroy(TFileValue* tf) { taosArrayDestroy(tf->tableId); + free(tf->colVal); free(tf); } static void tfileSerialTableIdsToBuf(char* buf, SArray* ids) { diff --git a/source/libs/index/test/indexTests.cc b/source/libs/index/test/indexTests.cc index 7bf40bc7b3..bdfb86ce17 100644 --- a/source/libs/index/test/indexTests.cc +++ b/source/libs/index/test/indexTests.cc @@ -457,7 +457,10 @@ TEST_F(IndexTFileEnv, test_tfile_write) { // taosArrayPush(data, &v4); fObj->Put(data); - for (size_t i = 0; i < taosArrayGetSize(data); i++) { destroyTFileValue(taosArrayGetP(data, i)); } + for (size_t i = 0; i < taosArrayGetSize(data); i++) { + // data + destroyTFileValue(taosArrayGetP(data, i)); + } taosArrayDestroy(data); std::string colName("voltage"); @@ -470,6 +473,7 @@ TEST_F(IndexTFileEnv, test_tfile_write) { fObj->Get(&query, result); assert(taosArrayGetSize(result) == 200); indexTermDestroy(term); + taosArrayDestroy(result); // tfileWriterDestroy(twrite); } @@ -534,6 +538,7 @@ TEST_F(IndexCacheEnv, cache_test) { SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); coj->Put(term, colId, version++, suid++); + indexTermDestroy(term); // indexTermDestry(term); } { @@ -541,24 +546,28 @@ TEST_F(IndexCacheEnv, cache_test) { SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); coj->Put(term, colId, version++, suid++); + indexTermDestroy(term); } { std::string colVal("v2"); SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); coj->Put(term, colId, version++, suid++); + indexTermDestroy(term); } { std::string colVal("v3"); SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); coj->Put(term, colId, version++, suid++); + indexTermDestroy(term); } { std::string colVal("v3"); SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); coj->Put(term, colId, version++, suid++); + indexTermDestroy(term); } coj->Debug(); std::cout << "--------first----------" << std::endl; @@ -567,12 +576,14 @@ TEST_F(IndexCacheEnv, cache_test) { SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); coj->Put(term, othColId, version++, suid++); + indexTermDestroy(term); } { std::string colVal("v4"); SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); coj->Put(term, othColId, version++, suid++); + indexTermDestroy(term); } coj->Debug(); std::cout << "--------second----------" << std::endl; @@ -583,6 +594,7 @@ TEST_F(IndexCacheEnv, cache_test) { SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); coj->Put(term, colId, version++, suid++); + indexTermDestroy(term); } } coj->Debug(); @@ -598,6 +610,9 @@ TEST_F(IndexCacheEnv, cache_test) { coj->Get(&query, colId, 10000, ret, &valType); std::cout << "size : " << taosArrayGetSize(ret) << std::endl; assert(taosArrayGetSize(ret) == 4); + taosArrayDestroy(ret); + + indexTermDestroy(term); } { std::string colVal("v2"); @@ -609,6 +624,9 @@ TEST_F(IndexCacheEnv, cache_test) { coj->Get(&query, colId, 10000, ret, &valType); assert(taosArrayGetSize(ret) == 1); + taosArrayDestroy(ret); + + indexTermDestroy(term); } } class IndexObj { @@ -678,13 +696,16 @@ class IndexObj { SArray* result = (SArray*)taosArrayInit(1, sizeof(uint64_t)); if (Search(mq, result) == 0) { std::cout << "search one successfully" << std::endl; } - return taosArrayGetSize(result); + int sz = taosArrayGetSize(result); + indexMultiTermQueryDestroy(mq); + taosArrayDestroy(result); + return sz; // assert(taosArrayGetSize(result) == targetSize); } void PutOne(const std::string& colName, const std::string& colVal) { + SIndexMultiTerm* terms = indexMultiTermCreate(); SIndexTerm* term = indexTermCreate(0, ADD_VALUE, TSDB_DATA_TYPE_BINARY, colName.c_str(), colName.size(), colVal.c_str(), colVal.size()); - SIndexMultiTerm* terms = indexMultiTermCreate(); indexMultiTermAdd(terms, term); Put(terms, 10); indexMultiTermDestroy(terms); @@ -783,6 +804,8 @@ TEST_F(IndexEnv2, testIndexOpen) { index->Search(mq, result); std::cout << "target size: " << taosArrayGetSize(result) << std::endl; assert(taosArrayGetSize(result) == 400); + taosArrayDestroy(result); + indexMultiTermQueryDestroy(mq); } }