Skip to content

Commit

Permalink
Merge pull request #6 from nvidianz/fix-double-free
Browse files Browse the repository at this point in the history
Fixed memory leaks in tests
  • Loading branch information
ZiyueXu77 authored May 17, 2024
2 parents 47176d8 + 89da71e commit 4f96a14
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 11 deletions.
2 changes: 1 addition & 1 deletion dmlc-core
5 changes: 4 additions & 1 deletion src/processing/plugins/mock_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ void* MockProcessor::HandleGHPairs(std::size_t *size, void *buffer, std::size_t
}
}

return buffer;
auto result = malloc(buf_size);
memcpy(result, buffer, buf_size);

return result;
}

void *MockProcessor::ProcessAggregation(std::size_t *size, std::map<int, std::vector<int>> nodes) {
Expand Down
18 changes: 15 additions & 3 deletions src/processing/plugins/mock_processor.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,27 @@ class MockProcessor: public processing::Processor {
std::vector<int> slots_;

public:

~MockProcessor() {
if (gh_pairs_) {
gh_pairs_->clear();
delete gh_pairs_;
}
}

void Initialize(bool active, std::map<std::string, std::string> params) override {
this->active_ = active;
this->params_ = &params;
}

void Shutdown() override {
this->gh_pairs_ = nullptr;
this->cuts_.clear();
this->slots_.clear();
if (gh_pairs_) {
gh_pairs_->clear();
delete gh_pairs_;
}
gh_pairs_ = nullptr;
cuts_.clear();
slots_.clear();
}

void FreeBuffer(void *buffer) override {
Expand Down
6 changes: 6 additions & 0 deletions src/processing/processor.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ const char kLoadFunc[] = "LoadProcessor";
/*! \brief An processor interface to handle tasks that require external library through plugins */
class Processor {
public:
/*!
* \brief Virtual destructor
*
*/
virtual ~Processor() = default;

/*!
* \brief Initialize the processor
*
Expand Down
8 changes: 6 additions & 2 deletions src/processing/processor_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,13 @@ namespace processing {

void ProcessorLoader::unload() {
#if defined(_WIN32)
FreeLibrary(handle_);
if (handle_) {
FreeLibrary(handle_);
}
#else
dlclose(handle_);
if (handle_) {
dlclose(handle_);
}
#endif
}
} // namespace processing
11 changes: 8 additions & 3 deletions tests/cpp/processing/test_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ class ProcessorTest : public testing::Test {
auto loader = processing::ProcessorLoader();
processor_ = loader.load(processing::kMockProcessor);
processor_->Initialize(true, {});
loader.unload();
}

void TearDown() override {
processor_->Shutdown();
delete processor_;
processor_ = nullptr;
}

Expand Down Expand Up @@ -62,13 +64,13 @@ TEST_F(ProcessorTest, TestGHEncoding) {
ASSERT_EQ(0, memcmp(buffer, new_buffer, buf_size));

// Clean up
free(buffer);
free(new_buffer);
processor_->FreeBuffer(buffer);
processor_->FreeBuffer(new_buffer);
}

TEST_F(ProcessorTest, TestAggregation) {
size_t buf_size;
processor_->ProcessGHPairs(&buf_size, gh_pairs_); // Pass the GH pairs to the plugin
auto gh_buffer = processor_->ProcessGHPairs(&buf_size, gh_pairs_); // Pass the GH pairs to the plugin

processor_->InitAggregationContext(cuts_, slots_);
auto buffer = processor_->ProcessAggregation(&buf_size, nodes_);
Expand All @@ -87,4 +89,7 @@ TEST_F(ProcessorTest, TestAggregation) {
for (size_t i = 0; i < histos.size(); ++i) {
EXPECT_NEAR(expected_result[i], histos[i], kError) << "Histogram differs at index " << i;
}

processor_->FreeBuffer(buffer);
processor_->FreeBuffer(gh_buffer);
}

0 comments on commit 4f96a14

Please sign in to comment.