Merge pull request #1482 from LLNL/bugfix/yang39/mem-leak

Fix a memory leak in axom::Array copy constructor

Author: publixsubfan

RELEASE-NOTES.md

@@ -43,6 +43,7 @@ to use Open CASCADE's file I/O capabilities in support of Quest applications.
 - Fixes compilation issue with [email protected] on 32-bit Windows configurations. 
 This required a [RAJA fix to avoid 64-bit intrinsics](https://github.com/LLNL/RAJA/pull/1746), 
 as well as support for 32-bit `Word`s in Slam's `BitSet` class.
+- Fixes a memory leak in `axom::Array` copy constructor.
 
 ## [Version 0.10.1] - Release date 2024-10-22
 

src/axom/core/Array.hpp

@@ -1139,7 +1139,8 @@ AXOM_HOST_DEVICE Array<T, DIM, SPACE>::Array(const Array& other)
   assert(false);
   #endif
 #else
-  initialize(other.size(), other.capacity());
+  this->setCapacity(other.capacity());
+  m_executeOnGPU = axom::isDeviceAllocator(m_allocator_id);
   // Use fill_range to ensure that copy constructors are invoked for each
   // element.
   MemorySpace srcSpace = SPACE;
@@ -1149,9 +1150,10 @@ AXOM_HOST_DEVICE Array<T, DIM, SPACE>::Array(const Array& other)
   }
   OpHelper {m_allocator_id, m_executeOnGPU}.fill_range(m_data,
                                                        0,
-                                                       m_num_elements,
+                                                       other.size(),
                                                        other.data(),
                                                        srcSpace);
+  this->updateNumElements(other.size());
 #endif
 }
 

src/axom/core/tests/core_array.hpp

@@ -2389,102 +2389,52 @@ TEST(core_array, reserve_nontrivial_reloc_um)
 }
 #endif
 
-// Regression test for a memory leak in axom::Array's copy/move/initialization
-template <typename T1, typename T2>
-auto make_arr_data(int SZ1, int SZ2)
-  -> std::pair<axom::Array<axom::Array<T1>>, axom::Array<axom::Array<T2>>>
+class AllocatingDefaultInit
 {
-  using Arr1 = axom::Array<axom::Array<T1>>;
-  using Arr2 = axom::Array<axom::Array<T2>>;
+public:
+  AllocatingDefaultInit() { m_value.resize(32); }
 
-  T1 val1 {};
-  T2 val2 {};
-
-  Arr1 arr1(SZ1, 2 * SZ1);
-  for(int i = 0; i < SZ1; ++i)
-  {
-    arr1[i].resize(i + 10);
-    arr1[i].shrink();
-    arr1[i].push_back(val1);
-  }
-
-  Arr2 arr2(SZ2, 2 * SZ2);
-  for(int i = 0; i < SZ2; ++i)
-  {
-    arr2[i].resize(i + 10);
-    arr2[i].shrink();
-    arr2[i].push_back(val2);
-  }
-  arr2.shrink();
-
-  return {std::move(arr1), std::move(arr2)};
-}
+private:
+  axom::Array<int> m_value;
+};
 
-TEST(core_array, regression_array_move)
+TEST(core_array, array_ctors_leak)
 {
-  using T1 = int;
-  using Arr1 = axom::Array<axom::Array<T1>>;
-
-  using T2 = NonTriviallyRelocatable;
-  using Arr2 = axom::Array<axom::Array<T2>>;
-
-  using PArrArr = std::pair<Arr1, Arr2>;
+  using Array = axom::Array<AllocatingDefaultInit>;
 
+  // Initialize test array
   constexpr int SZ1 = 20;
-  constexpr int SZ2 = 30;
+  Array data(SZ1);
 
+  // axom::Array copy constructor
   {
-    PArrArr pr;
-    pr = make_arr_data<T1, T2>(SZ1, SZ2);
+    Array arr_copy {data};
 
-    EXPECT_EQ(SZ1, pr.first.size());
-    EXPECT_EQ(2 * SZ1, pr.first.capacity());
-
-    EXPECT_EQ(SZ2, pr.second.size());
-    EXPECT_EQ(SZ2, pr.second.capacity());
+    EXPECT_EQ(SZ1, arr_copy.size());
   }
 
+  // axom::Array copy assignment operator
   {
-    auto pr = make_arr_data<T1, T2>(SZ1, SZ2);
-
-    EXPECT_EQ(SZ1, pr.first.size());
-    EXPECT_EQ(2 * SZ1, pr.first.capacity());
+    Array arr_copy {SZ1 + 10};
+    arr_copy = data;
 
-    EXPECT_EQ(SZ2, pr.second.size());
-    EXPECT_EQ(SZ2, pr.second.capacity());
+    EXPECT_EQ(SZ1, arr_copy.size());
   }
 
+  // axom::Array move constructor
   {
-    auto pr = make_arr_data<T1, T2>(SZ1, SZ2);
-    EXPECT_EQ(SZ1, pr.first.size());
-    EXPECT_EQ(2 * SZ1, pr.first.capacity());
-    EXPECT_EQ(SZ2, pr.second.size());
-    EXPECT_EQ(SZ2, pr.second.capacity());
+    Array tmp_move_from {data};
+    Array arr_move {std::move(tmp_move_from)};
 
-    pr = make_arr_data<T1, T2>(SZ2, SZ1);
-    EXPECT_EQ(SZ2, pr.first.size());
-    EXPECT_EQ(2 * SZ2, pr.first.capacity());
-    EXPECT_EQ(SZ1, pr.second.size());
-    EXPECT_EQ(SZ1, pr.second.capacity());
+    EXPECT_EQ(SZ1, arr_move.size());
   }
 
-  // lots of copy and move assignments and constructions
+  // axom::Array move assignment operator
   {
-    auto pr = make_arr_data<T1, T2>(SZ1, SZ2);
-    pr = make_arr_data<T1, T2>(5, 7);
-
-    auto pr2 = std::move(pr);
-    auto pr3 = pr2;
-
-    pr2 = make_arr_data<T1, T2>(13, 17);
-
-    auto pr4 {make_arr_data<T1, T2>(33, 44)};
-
-    auto pr5(std::move(pr4));
+    Array tmp_move_from {data};
+    Array arr_move {SZ1 + 10};
+    arr_move = std::move(tmp_move_from);
 
-    EXPECT_EQ(33, pr5.first.size());
-    EXPECT_EQ(66, pr5.first.capacity());
-    EXPECT_EQ(44, pr5.second.size());
-    EXPECT_EQ(44, pr5.second.capacity());
+    EXPECT_EQ(SZ1, arr_move.size());
   }
 }