From 18902be6358e54a65c166a601be20ab7cc1b84c9 Mon Sep 17 00:00:00 2001 From: Jinzhe Zeng Date: Thu, 14 Dec 2023 19:58:35 -0500 Subject: [PATCH] fix memory leaks related to `char*` (#3063) Make sure no leaks reported from `string_to_char` by LeakSanitizer. Most memory leaks happen only once, but `DP_CHECK_OK` causes 1 byte leaking every step, meaning 10 MB leaks for 10 million steps. --------- Signed-off-by: Jinzhe Zeng --- source/api_c/include/c_api.h | 7 +++++++ source/api_c/include/deepmd.hpp | 18 ++++++++++++------ source/api_c/src/c_api.cc | 2 ++ source/api_c/tests/test_deeppot_a.cc | 1 + 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/source/api_c/include/c_api.h b/source/api_c/include/c_api.h index b0c030962a..d05f790bf9 100644 --- a/source/api_c/include/c_api.h +++ b/source/api_c/include/c_api.h @@ -1271,6 +1271,13 @@ void DP_SelectMapInt(const int* in, const int nall2, int* out); +/** + * @brief Destroy a char array. + * + * @param c_str The char array. + */ +void DP_DeleteChar(const char* c_str); + #ifdef __cplusplus } /* end extern "C" */ #endif diff --git a/source/api_c/include/deepmd.hpp b/source/api_c/include/deepmd.hpp index 90c1c1c918..4a376e0bec 100644 --- a/source/api_c/include/deepmd.hpp +++ b/source/api_c/include/deepmd.hpp @@ -35,10 +35,14 @@ struct deepmd_exception : public std::runtime_error { /** * @brief Check if any exceptions throw in the C++ API. Throw if possible. */ -#define DP_CHECK_OK(check_func, dp) \ - const char *err_msg = check_func(dp); \ - if (std::strlen(err_msg)) \ - throw deepmd::hpp::deepmd_exception(std::string(err_msg)); +#define DP_CHECK_OK(check_func, dp) \ + const char *err_msg = check_func(dp); \ + if (std::strlen(err_msg)) { \ + std::string err_msg_str = std::string(err_msg); \ + DP_DeleteChar(err_msg); \ + throw deepmd::hpp::deepmd_exception(err_msg_str); \ + } \ + DP_DeleteChar(err_msg); template inline void _DP_DeepPotCompute(DP_DeepPot *dp, @@ -1019,7 +1023,7 @@ class DeepPot { void get_type_map(std::string &type_map) { const char *type_map_c = DP_DeepPotGetTypeMap(dp); type_map.assign(type_map_c); - delete[] type_map_c; + DP_DeleteChar(type_map_c); }; /** * @brief Print the summary of DeePMD-kit, including the version and the build @@ -1864,7 +1868,7 @@ class DeepTensor { void get_type_map(std::string &type_map) { const char *type_map_c = DP_DeepTensorGetTypeMap(dt); type_map.assign(type_map_c); - delete[] type_map_c; + DP_DeleteChar(type_map_c); }; private: @@ -2009,9 +2013,11 @@ void inline read_file_to_string(std::string model, std::string &file_content) { if (size < 0) { // negtive size indicates error std::string error_message = std::string(c_file_content, -size); + DP_DeleteChar(c_file_content); throw deepmd::hpp::deepmd_exception(error_message); } file_content = std::string(c_file_content, size); + DP_DeleteChar(c_file_content); }; /** diff --git a/source/api_c/src/c_api.cc b/source/api_c/src/c_api.cc index 9d1ed7d323..935e812cf0 100644 --- a/source/api_c/src/c_api.cc +++ b/source/api_c/src/c_api.cc @@ -1421,4 +1421,6 @@ void DP_SelectMapInt(const int* in, } } +void DP_DeleteChar(const char* c_str) { delete[] c_str; } + } // extern "C" diff --git a/source/api_c/tests/test_deeppot_a.cc b/source/api_c/tests/test_deeppot_a.cc index 50e8131cc0..63f53e16e9 100644 --- a/source/api_c/tests/test_deeppot_a.cc +++ b/source/api_c/tests/test_deeppot_a.cc @@ -172,6 +172,7 @@ TEST_F(TestInferDeepPotA, type_map) { const char* type_map = DP_DeepPotGetTypeMap(dp); char expected_type_map[] = "O H"; EXPECT_EQ(strcmp(type_map, expected_type_map), 0); + DP_DeleteChar(type_map); } class TestInferDeepPotANoPBC : public ::testing::Test {