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

Memory leak Table/AlignedVector #12998

Closed
peterrum opened this issue Nov 26, 2021 · 6 comments · Fixed by #13005
Closed

Memory leak Table/AlignedVector #12998

peterrum opened this issue Nov 26, 2021 · 6 comments · Fixed by #13005
Labels

Comments

@peterrum
Copy link
Member

as pointed out in #12993

@peterrum peterrum added the Bug label Nov 26, 2021
@pcafrica
Copy link
Contributor

pcafrica commented Nov 27, 2021

This is the most minimal working example that I could write to reproduce the memory leak.

It seems to first appear when cloning a Table of objects containing another Table (both non-empty, whatever the number of dimensions or the size).

#include <deal.II/base/table.h>

#include <iostream>
#include <memory>

using namespace dealii;

class TableWrapper
{
public:
  TableWrapper()
    : table({1})
  {}

  Table<1, double> table;
};

int
main()
{
  Table<1, TableWrapper> table_of_tables({1});
  table_of_tables[0].table.reinit({1});

  auto clone = std::make_unique<Table<1, TableWrapper>>(table_of_tables);

  std::cout << clone->size() << std::endl;

  return 0;
}

The output of valgrind --leak-check=full ./memory_leak is:

==7193== 8 bytes in 1 blocks are definitely lost in loss record 2 of 10
==7193==    at 0x4844990: memalign (in /u/sw/toolchains/gcc-glibc/11/base/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==7193==    by 0x4844AEE: posix_memalign (in /u/sw/toolchains/gcc-glibc/11/base/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==7193==    by 0x15EE0007: dealii::Utilities::System::posix_memalign(void**, unsigned long, unsigned long) (utilities.cc:1053)
==7193==    by 0x423328: dealii::AlignedVector<double>::reserve(unsigned long) (aligned_vector.h:1044)
==7193==    by 0x4222B2: dealii::AlignedVector<double>::resize(unsigned long) (aligned_vector.h:981)
==7193==    by 0x421809: dealii::TableBase<1, double>::reinit(dealii::TableIndices<1> const&, bool) (table.h:2411)
==7193==    by 0x420D7C: dealii::TableBase<1, double>::TableBase(dealii::TableIndices<1> const&) (table.h:2128)
==7193==    by 0x4206A8: dealii::Table<1, double>::Table(unsigned long) (table.h:2622)
==7193==    by 0x41FC56: TableWrapper::TableWrapper() (memory_leak.cc:12)
==7193==    by 0x425B4E: dealii::internal::AlignedVectorDefaultInitialize<TableWrapper, true>::default_construct_or_assign(unsigned long, unsigned long, std::integral_constant<bool, true>) const (aligned_vector.h:814)
==7193==    by 0x4250B8: dealii::internal::AlignedVectorDefaultInitialize<TableWrapper, true>::apply_to_subrange(unsigned long, unsigned long) const (aligned_vector.h:790)
==7193==    by 0x423F28: dealii::internal::AlignedVectorDefaultInitialize<TableWrapper, true>::AlignedVectorDefaultInitialize(unsigned long, TableWrapper*) (aligned_vector.h:769)

@pcafrica
Copy link
Contributor

pcafrica commented Nov 27, 2021

The interesting thing is that the following code (which is equivalent to the one above but without the intermediate wrapper class) does not leak:

int
main()
{
  Table<1, Table<1, double>> table_of_tables({1});
  table_of_tables[0].reinit({1});

  auto clone = std::make_unique<Table<1, Table<1, double>>>(table_of_tables);

  std::cout << clone->size() << std::endl;

  return 0;
}

This confirms that the issue is related to memory alignment, doesn't it?

@bangerth
Copy link
Member

I'll take a look in the next couple of days.

@pcafrica
Copy link
Contributor

pcafrica commented Nov 28, 2021

I'll take a look in the next couple of days.
@bangerth I would be glad to help, if I can.

@bangerth
Copy link
Member

I think you've done all you could with creating a small test case :-) I just need to get back to work -- the past week was our Thanksgiving break, and I tried to actually take time off from work.

@bangerth
Copy link
Member

For reference, here is a test that outputs the problem more directly:

// ---------------------------------------------------------------------
//
// Copyright (C) 2021 by the deal.II authors
//
// This file is part of the deal.II library.
//
// The deal.II library is free software; you can use it, redistribute
// it, and/or modify it under the terms of the GNU Lesser General
// Public License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
// The full text of the license can be found in the file LICENSE.md at
// the top level directory of deal.II.
//
// ---------------------------------------------------------------------


// Check for a bug in memory management in class Table (which uses
// AlignedVector) that had crept in at some point.

#include <deal.II/base/logstream.h>
#include <deal.II/base/table.h>

#include "../tests.h"


int object_number     = 0;
int objects_destroyed = 0;

class C
{
public:
  C()
  {
    object_number = ::object_number++;
    deallog << "Default constructor. Object number " << object_number
            << std::endl;
  }

  C(const C &c)
  {
    object_number = ::object_number++;
    deallog << "copy constructor from " << c.object_number << ". Object number "
            << object_number << std::endl;
  }

  C(const C &&c)
  {
    object_number = ::object_number++;
    deallog << "move constructor from " << c.object_number << ". Object number "
            << object_number << std::endl;
  }

  C &
  operator=(const C &c)
  {
    deallog << "copy operator called for " << object_number << " <- "
            << c.object_number << std::endl;
    return *this;
  }

  C &
  operator=(const C &&c)
  {
    deallog << "move operator called for " << object_number << " <- std::move("
            << c.object_number << ')' << std::endl;
    return *this;
  }


  ~C()
  {
    deallog << "destructor. Object number " << object_number << std::endl;
    ++objects_destroyed;
  }

private:
  unsigned int object_number;
};



class TableWrapper
{
public:
  TableWrapper()
    : table({1})
  {}

  Table<1, C> table;
};


void
test()
{
  Table<1, TableWrapper> table_of_tables({1});
  table_of_tables[0].table.reinit({1});

  // Copy the object, then destroy the copy again.
  auto clone = std::make_unique<Table<1, TableWrapper>>(table_of_tables);
}



int
main(int argc, char **argv)
{
  initlog();

  test();

  deallog << "Objects created: " << object_number << std::endl;
  deallog << "Objects destroyed: " << objects_destroyed << std::endl;
}

with output:

DEAL::Default constructor. Object number 0
DEAL::Default constructor. Object number 1
DEAL::move operator called for 0 <- std::move(1)
DEAL::destructor. Object number 1
DEAL::Default constructor. Object number 2
DEAL::destructor. Object number 2
DEAL::Default constructor. Object number 3
DEAL::Default constructor. Object number 4
DEAL::destructor. Object number 4
DEAL::Default constructor. Object number 5
DEAL::copy constructor from 0. Object number 6
DEAL::destructor. Object number 6
DEAL::destructor. Object number 0
DEAL::Objects created: 7
DEAL::Objects destroyed: 5

Objects number 3 and 5 are never destroyed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants