From c968df4b618fd5e00af4062f1a54574b345fcfe8 Mon Sep 17 00:00:00 2001 From: Seunghwa Kang <45857425+seunghwak@users.noreply.github.com> Date: Mon, 3 Jan 2022 12:20:35 -0800 Subject: [PATCH] Fix comms memory leak (#436) Close #435 - [x] Add a virtual destructor to comms_iface (to invoke std_comms/mpi_comms destructors when std_comms/mpi_comms objects are deleted through a comms_iface pointer). - [x] Set `owns_mpi_comms` to false in `initialize_mpi_comms` as this takes an already initialized `comm` as an input parameter so `comm` may better be destroyed by the caller (and no need to destroy if `comm` is MPI_COMM_WORLD, just calling MPI_Finalize will be sufficient). Authors: - Seunghwa Kang (https://github.com/seunghwak) Approvers: - Corey J. Nolet (https://github.com/cjnolet) - Chuck Hastings (https://github.com/ChuckHastings) URL: https://github.com/rapidsai/raft/pull/436 --- cpp/include/raft/comms/comms.hpp | 2 ++ cpp/include/raft/comms/mpi_comms.hpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/include/raft/comms/comms.hpp b/cpp/include/raft/comms/comms.hpp index 68b8e723e9..0de84117e0 100644 --- a/cpp/include/raft/comms/comms.hpp +++ b/cpp/include/raft/comms/comms.hpp @@ -90,6 +90,8 @@ constexpr datatype_t get_type() class comms_iface { public: + virtual ~comms_iface() {} + virtual int get_size() const = 0; virtual int get_rank() const = 0; diff --git a/cpp/include/raft/comms/mpi_comms.hpp b/cpp/include/raft/comms/mpi_comms.hpp index 5cdde29db5..432f250b59 100644 --- a/cpp/include/raft/comms/mpi_comms.hpp +++ b/cpp/include/raft/comms/mpi_comms.hpp @@ -439,7 +439,7 @@ class mpi_comms : public comms_iface { inline void initialize_mpi_comms(handle_t* handle, MPI_Comm comm) { auto communicator = - std::make_shared(std::unique_ptr(new mpi_comms(comm, true))); + std::make_shared(std::unique_ptr(new mpi_comms(comm, false))); handle->set_comms(communicator); };