Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mpi_type for std::pair leads to memory leaks #103

Open
lindsayad opened this issue Aug 15, 2023 · 3 comments
Open

mpi_type for std::pair leads to memory leaks #103

lindsayad opened this issue Aug 15, 2023 · 3 comments

Comments

@lindsayad
Copy link

This code in MPIWrapper.hpp leaks memory

  template<> inline MPI_Datatype mpi_type<std::pair<long int,long int>>() {
    static MPI_Datatype l_l_mpi_type = MPI_DATATYPE_NULL;
    if (l_l_mpi_type == MPI_DATATYPE_NULL) {
      MPI_Type_contiguous
        (2, strumpack::mpi_type<long int>(), &l_l_mpi_type);
      MPI_Type_commit(&l_l_mpi_type);
    }
    return l_l_mpi_type;
  }
  /** return MPI datatype for C++ std::pair<long long int,long long int> */
  template<> inline MPI_Datatype mpi_type<std::pair<long long int,long long int>>() {
    static MPI_Datatype ll_ll_mpi_type = MPI_DATATYPE_NULL;
    if (ll_ll_mpi_type == MPI_DATATYPE_NULL) {
      MPI_Type_contiguous
        (2, strumpack::mpi_type<long long int>(), &ll_ll_mpi_type);
      MPI_Type_commit(&ll_ll_mpi_type);
    }
    return ll_ll_mpi_type;
  }

It is hardly a significant leak, but it showed up in our valgrind testing when updating MOOSE to recent PETSc (CI job https://civet.inl.gov/job/1698426/ on idaholab/moose#23502). We had the same static design pattern in https://github.com/libmesh/TIMPI for a long time, figuring that the few leaked bytes weren't a big deal. But enough users complained that we ended up changing to a RAII type design (TIMPI code here).

Anyway this isn't a huge deal. We'll probably just add a suppression for this for now. Thanks for your solver work!

@pghysels
Copy link
Owner

Thank you for pointing this out. I'll need to think about how to fix this.
I tried to fix memory leaks from MPI_Type_commit (for instance in src/misc/Triplet.hpp), but I must have missed this.

@pghysels
Copy link
Owner

I think this fixes it: f301da7
I know I should add valgrind testing to the CI.

@lindsayad
Copy link
Author

Yep that does it!

cticenhour added a commit to cticenhour/moose that referenced this issue Aug 15, 2023
cticenhour added a commit to cticenhour/moose that referenced this issue Aug 15, 2023
cticenhour added a commit to cticenhour/moose that referenced this issue Aug 30, 2023
cticenhour added a commit to cticenhour/moose that referenced this issue Aug 30, 2023
cticenhour added a commit to cticenhour/moose that referenced this issue Aug 31, 2023
cticenhour added a commit to cticenhour/moose that referenced this issue Sep 6, 2023
cticenhour added a commit to cticenhour/moose that referenced this issue Sep 11, 2023
cticenhour added a commit to cticenhour/moose that referenced this issue Sep 15, 2023
cticenhour added a commit to cticenhour/moose that referenced this issue Sep 18, 2023
oanaoana pushed a commit to oanaoana/moose that referenced this issue Oct 19, 2023
oanaoana pushed a commit to oanaoana/moose that referenced this issue Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants