From b2eb9fe2a7dc2ea587061a155b60f0461b7dbaa6 Mon Sep 17 00:00:00 2001 From: Zhen Ye Date: Mon, 2 Sep 2024 15:59:04 +0800 Subject: [PATCH] fix: memory leak in unittest and open the USE_ASAN option when build unittest (#35855) issue: #35854 --------- Signed-off-by: chyezh --- internal/core/unittest/CMakeLists.txt | 2 +- internal/core/unittest/test_c_api.cpp | 5 +++++ internal/core/unittest/test_data_codec.cpp | 6 +++--- internal/core/unittest/test_sealed.cpp | 8 ++++---- internal/core/unittest/test_utils/c_api_test_utils.h | 3 +++ 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/internal/core/unittest/CMakeLists.txt b/internal/core/unittest/CMakeLists.txt index d442fa3f0f55f..4b999b4d7e5ce 100644 --- a/internal/core/unittest/CMakeLists.txt +++ b/internal/core/unittest/CMakeLists.txt @@ -117,7 +117,7 @@ endif() if (LINUX) message( STATUS "Building Milvus Unit Test on Linux") option(USE_ASAN "Whether to use AddressSanitizer" OFF) - if ( USE_ASAN AND false ) + if ( USE_ASAN ) message( STATUS "Building Milvus using AddressSanitizer") add_compile_options(-fno-stack-protector -fno-omit-frame-pointer -fno-var-tracking -fsanitize=address) add_link_options(-fno-stack-protector -fno-omit-frame-pointer -fno-var-tracking -fsanitize=address) diff --git a/internal/core/unittest/test_c_api.cpp b/internal/core/unittest/test_c_api.cpp index fdfb0422f2454..7aa8815f6f697 100644 --- a/internal/core/unittest/test_c_api.cpp +++ b/internal/core/unittest/test_c_api.cpp @@ -35,6 +35,7 @@ #include "segcore/reduce_c.h" #include "segcore/segment_c.h" #include "futures/Future.h" +#include "futures/future_c.h" #include "test_utils/DataGen.h" #include "test_utils/PbHelper.h" #include "test_utils/indexbuilder_test_utils.h" @@ -79,6 +80,8 @@ CRetrieve(CSegmentInterface c_segment, mu.lock(); auto [retrieveResult, status] = futurePtr->leakyGet(); + future_destroy(future); + if (status.error_code != 0) { return status; } @@ -104,6 +107,8 @@ CRetrieveByOffsets(CSegmentInterface c_segment, mu.lock(); auto [retrieveResult, status] = futurePtr->leakyGet(); + future_destroy(future); + if (status.error_code != 0) { return status; } diff --git a/internal/core/unittest/test_data_codec.cpp b/internal/core/unittest/test_data_codec.cpp index f456461fc01c9..7831454c6a03d 100644 --- a/internal/core/unittest/test_data_codec.cpp +++ b/internal/core/unittest/test_data_codec.cpp @@ -433,8 +433,8 @@ TEST(storage, InsertDataFloatNullable) { FixedVector data = {1, 2, 3, 4, 5}; auto field_data = milvus::storage::CreateFieldData(storage::DataType::FLOAT, true); - uint8_t* valid_data = new uint8_t[1]{0x13}; - field_data->FillFieldData(data.data(), valid_data, data.size()); + std::array valid_data = {0x13}; + field_data->FillFieldData(data.data(), valid_data.data(), data.size()); storage::InsertData insert_data(field_data); storage::FieldDataMeta field_data_meta{100, 101, 102, 103}; @@ -457,7 +457,7 @@ TEST(storage, InsertDataFloatNullable) { data = {1, 2, 0, 0, 5}; ASSERT_EQ(data, new_data); ASSERT_EQ(new_payload->get_null_count(), 2); - ASSERT_EQ(*new_payload->ValidData(), *valid_data); + ASSERT_EQ(*new_payload->ValidData(), valid_data[0]); } TEST(storage, InsertDataDouble) { diff --git a/internal/core/unittest/test_sealed.cpp b/internal/core/unittest/test_sealed.cpp index 6b77798c0dfb5..c27021f7147ab 100644 --- a/internal/core/unittest/test_sealed.cpp +++ b/internal/core/unittest/test_sealed.cpp @@ -2015,12 +2015,12 @@ TEST(Sealed, SkipIndexSkipUnaryRangeNullable) { //test for int64 std::vector int64s = {1, 2, 3, 4, 5}; - uint8_t* valid_data = new uint8_t[1]{0x03}; + std::array valid_data = {0x03}; FixedVector valid_data_ = {true, true, false, false, false}; auto int64s_field_data = storage::CreateFieldData(DataType::INT64, true, 1, 5); - int64s_field_data->FillFieldData(int64s.data(), valid_data, 5); + int64s_field_data->FillFieldData(int64s.data(), valid_data.data(), 5); segment->LoadPrimitiveSkipIndex(i64_fid, 0, DataType::INT64, @@ -2084,12 +2084,12 @@ TEST(Sealed, SkipIndexSkipBinaryRangeNullable) { //test for int64 std::vector int64s = {1, 2, 3, 4, 5}; - uint8_t* valid_data = new uint8_t[1]{0x03}; + std::array valid_data = {0x03}; FixedVector valid_data_ = {true, true, false, false, false}; auto int64s_field_data = storage::CreateFieldData(DataType::INT64, true, 1, 5); - int64s_field_data->FillFieldData(int64s.data(), valid_data, 5); + int64s_field_data->FillFieldData(int64s.data(), valid_data.data(), 5); segment->LoadPrimitiveSkipIndex(i64_fid, 0, DataType::INT64, diff --git a/internal/core/unittest/test_utils/c_api_test_utils.h b/internal/core/unittest/test_utils/c_api_test_utils.h index 34fe7d29cdf08..b83d6e5640b92 100644 --- a/internal/core/unittest/test_utils/c_api_test_utils.h +++ b/internal/core/unittest/test_utils/c_api_test_utils.h @@ -29,6 +29,7 @@ #include "segcore/reduce_c.h" #include "segcore/segment_c.h" #include "futures/Future.h" +#include "futures/future_c.h" #include "DataGen.h" #include "PbHelper.h" #include "c_api_test_utils.h" @@ -193,6 +194,8 @@ CSearch(CSegmentInterface c_segment, mu.lock(); auto [searchResult, status] = futurePtr->leakyGet(); + future_destroy(future); + if (status.error_code != 0) { return status; }