Skip to content

Commit

Permalink
Fix comms memory leak (#436)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
seunghwak authored Jan 3, 2022
1 parent ccd5d75 commit c968df4
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
2 changes: 2 additions & 0 deletions cpp/include/raft/comms/comms.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion cpp/include/raft/comms/mpi_comms.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

Expand Down

0 comments on commit c968df4

Please sign in to comment.