Skip to content

Commit

Permalink
fix memory leaks related to char* (#3063)
Browse files Browse the repository at this point in the history
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 <jinzhe.zeng@rutgers.edu>
  • Loading branch information
njzjz authored Dec 15, 2023
1 parent e048389 commit 18902be
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 6 deletions.
7 changes: 7 additions & 0 deletions source/api_c/include/c_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 12 additions & 6 deletions source/api_c/include/deepmd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename FPTYPE>
inline void _DP_DeepPotCompute(DP_DeepPot *dp,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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);
};

/**
Expand Down
2 changes: 2 additions & 0 deletions source/api_c/src/c_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1421,4 +1421,6 @@ void DP_SelectMapInt(const int* in,
}
}

void DP_DeleteChar(const char* c_str) { delete[] c_str; }

} // extern "C"
1 change: 1 addition & 0 deletions source/api_c/tests/test_deeppot_a.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 18902be

Please sign in to comment.