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: #436
rapidsai · Jan 3, 2022 · c968df4 · c968df4
1 parent ccd5d75
commit c968df4
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 1 deletion.
diff --git a/cpp/include/raft/comms/comms.hpp b/cpp/include/raft/comms/comms.hpp
@@ -90,6 +90,8 @@ constexpr datatype_t get_type<double>()
 
 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
@@ -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<comms_t>(std::unique_ptr<comms_iface>(new mpi_comms(comm, true)));
+    std::make_shared<comms_t>(std::unique_ptr<comms_iface>(new mpi_comms(comm, false)));
   handle->set_comms(communicator);
 };