]> https://gitweb.dealii.org/ - dealii.git/commitdiff
Fix memory leak in AlignedVector
authorMartin Kronbichler <kronbichler@lnm.mw.tum.de>
Fri, 8 Jan 2016 15:39:41 +0000 (16:39 +0100)
committerMartin Kronbichler <kronbichler@lnm.mw.tum.de>
Fri, 8 Jan 2016 15:39:41 +0000 (16:39 +0100)
include/deal.II/base/aligned_vector.h
include/deal.II/base/table.h
tests/base/table_06.cc [new file with mode: 0644]
tests/base/table_06.output [new file with mode: 0644]

index 6935823d66ec07ac1bce553c25b1089058755997..5c81dc0138de6fbbc9d064600c3088b05a79d483 100644 (file)
@@ -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<T>::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<end; ++i)
           {
             // initialize memory (copy construct), and destruct
@@ -342,7 +343,7 @@ namespace internal
   private:
     T *source_;
     T *destination_;
-    const bool copy_only_;
+    const bool copy_source_;
   };
 
   /**
@@ -361,11 +362,13 @@ namespace internal
      */
     AlignedVectorSet (const std::size_t size,
                       const T &element,
-                      T *destination)
+                      T *destination,
+                      const bool initialize_memory)
       :
       element_ (element),
       destination_ (destination),
-      trivial_element (false)
+      trivial_element (false),
+      initialize_memory (initialize_memory)
     {
       if (size == 0)
         return;
@@ -403,15 +406,19 @@ namespace internal
       // non-trivial).
       if (std_cxx11::is_trivial<T>::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<end; ++i)
           new (&destination_[i]) T(element_);
+      else
+        for (std::size_t i=begin; i<end; ++i)
+          destination_[i] = element_;
     }
 
     const T &element_;
     mutable T *destination_;
     bool trivial_element;
+    const bool initialize_memory;
   };
 
 } // end of namespace internal
@@ -486,10 +493,22 @@ AlignedVector<T>::operator = (const AlignedVector<T> &vec)
 template < class T >
 inline
 void
-AlignedVector<T>::resize_fast (const size_type size)
+AlignedVector<T>::resize_fast (const size_type size_in)
 {
-  reserve (size);
-  _end_data = _data + size;
+  const size_type old_size = size();
+  if (std_cxx11::is_trivial<T>::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<T>::value == false && size_in > old_size)
+    dealii::internal::AlignedVectorSet<T> (size_in-old_size, T(), _data+old_size, true);
 }
 
 
@@ -507,12 +526,12 @@ AlignedVector<T>::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<T> (size_in-old_size, init, _data+old_size);
+    dealii::internal::AlignedVectorSet<T> (size_in-old_size, init, _data+old_size, true);
 }
 
 
@@ -548,9 +567,11 @@ AlignedVector<T>::reserve (const size_type size_alloc)
       if (_end_data != _data)
         {
           dealii::internal::AlignedVectorMove<T>(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<T>::fill (const T &value)
 {
-  dealii::internal::AlignedVectorSet<T> (size(), value, _data);
+  dealii::internal::AlignedVectorSet<T> (size(), value, _data, false);
 }
 
 
index 1c56f69437e05a3991e9daae4c73eded3a3b28f3..54c8f804d6ad5acc485f99d061dd407612207780 100644 (file)
@@ -1631,8 +1631,8 @@ TableBase<N,T>::TableBase (const TableBase<N,T> &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 (file)
index 0000000..ca1ba8c
--- /dev/null
@@ -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<N,ComplicatedType> 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 <deal.II/base/table.h>
+
+// 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<unsigned int> 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 (file)
index 0000000..d3d1f15
--- /dev/null
@@ -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

In the beginning the Universe was created. This has made a lot of people very angry and has been widely regarded as a bad move.

Douglas Adams


Typeset in Trocchi and Trocchi Bold Sans Serif.