Skip to content

Commit

Permalink
Move to a process global for test MemoryTracker (#4739)
Browse files Browse the repository at this point in the history
I've seen about three or four different test failures due to
`create_test_memory_tracker` returning new instances which leads to life
time issues with the memory resources. This changes to a single process
singleton to avoid the issue altogether.

Once we merge the current PRs that are all using
`create_test_memory_tracker` I'll follow up with a PR to rename all of
those instances to `get_test_memory_tracker` and remove the
`create_test_memory_tracker` to prevent further use.

---
TYPE: NO_HISTORY
DESC: Move to a process global for test MemoryTracker
  • Loading branch information
davisp authored Feb 18, 2024
1 parent c1e3427 commit c6c7615
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
14 changes: 12 additions & 2 deletions test/support/src/mem_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,25 @@

namespace tiledb::test {

shared_ptr<sm::MemoryTracker> create_test_memory_tracker() {
shared_ptr<sm::MemoryTracker> get_test_memory_tracker() {
class MemoryTrackerCreator : public sm::MemoryTracker {
public:
MemoryTrackerCreator()
: sm::MemoryTracker() {
}

static shared_ptr<MemoryTracker> get_instance() {
static shared_ptr<MemoryTrackerCreator> tracker{
new MemoryTrackerCreator()};
return tracker;
}
};

return make_shared<MemoryTrackerCreator>(HERE());
return MemoryTrackerCreator::get_instance();
}

shared_ptr<sm::MemoryTracker> create_test_memory_tracker() {
return get_test_memory_tracker();
}

} // namespace tiledb::test
12 changes: 12 additions & 0 deletions test/support/src/mem_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@

namespace tiledb::test {

/**
* Helper function get the test instance of a shared_ptr<MemoryTracker>
*
* This is the preferred function. The create_test_memory_tracker will be
* replaced shortly and only serves as a proxy to this function while we
* transition the first few PRs to use this new function.
*
* The reasoning here is that creating memory trackers has turned out to be a
* bit of a footgun with lifetime issues.
*/
shared_ptr<sm::MemoryTracker> get_test_memory_tracker();

/**
* Helper function to create test instances of shared_ptr<MemoryTracker>
*/
Expand Down

0 comments on commit c6c7615

Please sign in to comment.