From a89bad269b58fd21d71bf720631f7924a208f1d9 Mon Sep 17 00:00:00 2001 From: Martin Kronbichler Date: Fri, 8 Jan 2016 16:39:41 +0100 Subject: [PATCH] Fix memory leak in AlignedVector --- include/deal.II/base/aligned_vector.h | 57 +++++++++++++------ include/deal.II/base/table.h | 2 +- tests/base/table_06.cc | 79 +++++++++++++++++++++++++++ tests/base/table_06.output | 24 ++++++++ 4 files changed, 143 insertions(+), 19 deletions(-) create mode 100644 tests/base/table_06.cc create mode 100644 tests/base/table_06.output diff --git a/include/deal.II/base/aligned_vector.h b/include/deal.II/base/aligned_vector.h index 6935823d66..5c81dc0138 100644 --- a/include/deal.II/base/aligned_vector.h +++ b/include/deal.II/base/aligned_vector.h @@ -292,16 +292,17 @@ namespace internal * Constructor. Issues a parallel call if there are sufficiently many * elements, otherwise work in serial. Copies the data from source to * destination and then calls destructor on the source. If the optional - * argument is set to true, the source is left untouched instead. + * argument copy_source is set to true, the source is left untouched + * instead. */ AlignedVectorMove (T *source_begin, T *source_end, T *destination, - bool copy_only = false) + bool copy_source) : source_ (source_begin), destination_ (destination), - copy_only_ (copy_only) + copy_source_ (copy_source) { Assert (source_end >= source_begin, ExcInternalError()); const std::size_t size = source_end - source_begin; @@ -325,7 +326,7 @@ namespace internal if (std_cxx11::is_trivial::value == true) std::memcpy ((void *)(destination_+begin), source_+begin, (end-begin)*sizeof(T)); - else if (copy_only_ == false) + else if (copy_source_ == false) for (std::size_t i=begin; i::value == true && trivial_element) std::memset ((void *)(destination_+begin), 0, (end-begin)*sizeof(T)); - else - // initialize memory and set + else if (initialize_memory) + // initialize memory and set by copy constructor for (std::size_t i=begin; i::operator = (const AlignedVector &vec) template < class T > inline void -AlignedVector::resize_fast (const size_type size) +AlignedVector::resize_fast (const size_type size_in) { - reserve (size); - _end_data = _data + size; + const size_type old_size = size(); + if (std_cxx11::is_trivial::value == false && size_in < old_size) + { + // call destructor on fields that are released + while (_end_data != _data+size_in) + (--_end_data)->~T(); + } + reserve (size_in); + _end_data = _data + size_in; + + // need to still set the values in case the class is non-trivial because + // virtual classes etc. need to run their (default) constructor + if (std_cxx11::is_trivial::value == false && size_in > old_size) + dealii::internal::AlignedVectorSet (size_in-old_size, T(), _data+old_size, true); } @@ -507,12 +526,12 @@ AlignedVector::resize (const size_type size_in, while (_end_data != _data+size_in) (--_end_data)->~T(); } + reserve (size_in); + _end_data = _data + size_in; - resize_fast (size_in); - // now _size is set correctly, need to set the - // values + // finally set the desired init values if (size_in > old_size) - dealii::internal::AlignedVectorSet (size_in-old_size, init, _data+old_size); + dealii::internal::AlignedVectorSet (size_in-old_size, init, _data+old_size, true); } @@ -548,9 +567,11 @@ AlignedVector::reserve (const size_type size_alloc) if (_end_data != _data) { dealii::internal::AlignedVectorMove(new_data, new_data + old_size, - _data); + _data, false); free(new_data); } + else + Assert(new_data == 0, ExcInternalError()); } else if (size_alloc == 0) clear(); @@ -641,7 +662,7 @@ inline void AlignedVector::fill (const T &value) { - dealii::internal::AlignedVectorSet (size(), value, _data); + dealii::internal::AlignedVectorSet (size(), value, _data, false); } diff --git a/include/deal.II/base/table.h b/include/deal.II/base/table.h index 1c56f69437..54c8f804d6 100644 --- a/include/deal.II/base/table.h +++ b/include/deal.II/base/table.h @@ -1631,8 +1631,8 @@ TableBase::TableBase (const TableBase &src) : Subscriptor () { - values = src.values; reinit (src.table_size, true); + values = src.values; } diff --git a/tests/base/table_06.cc b/tests/base/table_06.cc new file mode 100644 index 0000000000..ca1ba8c87e --- /dev/null +++ b/tests/base/table_06.cc @@ -0,0 +1,79 @@ +// --------------------------------------------------------------------- +// +// Copyright (C) 1998 - 2015 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 at +// the top level of the deal.II distribution. +// +// --------------------------------------------------------------------- + + +// check that Table works properties (this really tests the +// data type underlying the values field of Table and that +// fill()/resize_fast() works properly). + +// NOTE: The number of calls to the constructor/destructor depends on the +// actual implementation of TableBase. When that is changed, this test will +// typically fail. When adjusting the output, make sure to check this test +// with valgrind + +#include "../tests.h" + +#include + +// make function virtual to ensure that the function table is correctly copied +class FunctionBase +{ +public: + ~FunctionBase() {} + + virtual void do_test() = 0; +}; + +class Function +{ +public: + Function () + : + size_ (2) + { + deallog << "Construct object" << std::endl; + } + + ~Function() + { + deallog << "Destruct with size " << vec.size() << std::endl; + } + + virtual void do_test() + { + vec.resize(size_++); + deallog << "Resize vector to " << vec.size() << std::endl; + } + +private: + unsigned int size_; + std::vector vec; +}; + +int main() +{ + initlog(); + dealii::Table<2,Function> table; + table.reinit(dealii::TableIndices<2>(2,1)); + table[1][0].do_test(); + table[0][0].do_test(); + table.reinit(dealii::TableIndices<2>(1,1), false); + table[0][0].do_test(); + table.reinit(dealii::TableIndices<2>(1,1), true); + table[0][0].do_test(); + table.reinit(0,0); + table.reinit(dealii::TableIndices<2>(2,2)); + table[0][1].do_test(); +} diff --git a/tests/base/table_06.output b/tests/base/table_06.output new file mode 100644 index 0000000000..d3d1f15dfd --- /dev/null +++ b/tests/base/table_06.output @@ -0,0 +1,24 @@ + +DEAL::Construct object +DEAL::Destruct with size 0 +DEAL::Construct object +DEAL::Destruct with size 0 +DEAL::Resize vector to 2 +DEAL::Resize vector to 2 +DEAL::Destruct with size 2 +DEAL::Construct object +DEAL::Destruct with size 0 +DEAL::Resize vector to 2 +DEAL::Resize vector to 3 +DEAL::Construct object +DEAL::Destruct with size 3 +DEAL::Destruct with size 0 +DEAL::Construct object +DEAL::Destruct with size 0 +DEAL::Construct object +DEAL::Destruct with size 0 +DEAL::Resize vector to 2 +DEAL::Destruct with size 0 +DEAL::Destruct with size 0 +DEAL::Destruct with size 2 +DEAL::Destruct with size 0 -- 2.39.5