Skip to content

Commit

Permalink
Fix possible memory leak in constructors (#3062)
Browse files Browse the repository at this point in the history
When a constructor throws an exception, it will not call the destructor
as the constructor is not finished. The memory leak happens here and is
detected by LeakSanitizer (thanks, @Cloudac7, for reminding me to use
it). We must catch the exception, delete the memory, and re-throw the
exception.

---------

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
njzjz and pre-commit-ci[bot] authored Dec 15, 2023
1 parent 1a7e252 commit e048389
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 5 deletions.
8 changes: 7 additions & 1 deletion source/api_cc/src/DataModifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ DipoleChargeModifier::DipoleChargeModifier(const std::string& model,
const int& gpu_rank,
const std::string& name_scope_)
: inited(false), name_scope(name_scope_), graph_def(new GraphDef()) {
init(model, gpu_rank, name_scope_);
try {
init(model, gpu_rank, name_scope_);
} catch (...) {
// Clean up and rethrow, as the destructor will not be called
delete graph_def;
throw;
}
}

DipoleChargeModifier::~DipoleChargeModifier() { delete graph_def; };
Expand Down
18 changes: 16 additions & 2 deletions source/api_cc/src/DeepPot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,13 @@ DeepPot::DeepPot(const std::string& model,
const int& gpu_rank,
const std::string& file_content)
: inited(false), init_nbor(false), graph_def(new GraphDef()) {
init(model, gpu_rank, file_content);
try {
init(model, gpu_rank, file_content);
} catch (...) {
// Clean up and rethrow, as the destructor will not be called
delete graph_def;
throw;
}
}

DeepPot::~DeepPot() { delete graph_def; }
Expand Down Expand Up @@ -1236,7 +1242,15 @@ DeepPotModelDevi::DeepPotModelDevi(
const int& gpu_rank,
const std::vector<std::string>& file_contents)
: inited(false), init_nbor(false), numb_models(0) {
init(models, gpu_rank, file_contents);
try {
init(models, gpu_rank, file_contents);
} catch (...) {
// Clean up and rethrow, as the destructor will not be called
for (unsigned ii = 0; ii < numb_models; ++ii) {
delete graph_defs[ii];
}
throw;
}
}

DeepPotModelDevi::~DeepPotModelDevi() {
Expand Down
8 changes: 7 additions & 1 deletion source/api_cc/src/DeepTensor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ DeepTensor::DeepTensor(const std::string &model,
const int &gpu_rank,
const std::string &name_scope_)
: inited(false), name_scope(name_scope_), graph_def(new GraphDef()) {
init(model, gpu_rank, name_scope_);
try {
init(model, gpu_rank, name_scope_);
} catch (...) {
// Clean up and rethrow, as the destructor will not be called
delete graph_def;
throw;
}
}

DeepTensor::~DeepTensor() { delete graph_def; }
Expand Down
20 changes: 19 additions & 1 deletion source/api_cc/tests/test_deepmd_exception.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
#include <string>
#include <vector>

#include "DataModifier.h"
#include "DeepPot.h"
#include "DeepTensor.h"
#include "errors.h"
TEST(TestDeepmdException, deepmdexception) {
std::string expected_error_message = "DeePMD-kit Error: unittest";
Expand All @@ -21,6 +23,22 @@ TEST(TestDeepmdException, deepmdexception) {
}
}

TEST(TestDeepmdException, deepmdexception_nofile) {
TEST(TestDeepmdException, deepmdexception_nofile_deeppot) {
ASSERT_THROW(deepmd::DeepPot("_no_such_file.pb"), deepmd::deepmd_exception);
}

TEST(TestDeepmdException, deepmdexception_nofile_deeppotmodeldevi) {
ASSERT_THROW(
deepmd::DeepPotModelDevi({"_no_such_file.pb", "_no_such_file.pb"}),
deepmd::deepmd_exception);
}

TEST(TestDeepmdException, deepmdexception_nofile_deeptensor) {
ASSERT_THROW(deepmd::DeepTensor("_no_such_file.pb"),
deepmd::deepmd_exception);
}

TEST(TestDeepmdException, deepmdexception_nofile_dipolechargemodifier) {
ASSERT_THROW(deepmd::DipoleChargeModifier("_no_such_file.pb"),
deepmd::deepmd_exception);
}

0 comments on commit e048389

Please sign in to comment.