Merge topic 'better_stealarray_api'

674fe1fb StealArray now returns the array and free function as a Pair.
Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !1664

docs/changelog/StorageBase-StealArray-returns-delete-function.md

+## `StorageBasic` StealArray() now provides delete function to new owner
+
+Memory that is stolen from VTK-m has to be freed correctly. This is required
+as the memory could have been allocated with `new`, `malloc` or even `cudaMallocManaged`.
+
+Previously it was very easy to transfer ownership of memory out of VTK-m and
+either fail to capture the free function, or ask for it after the transfer
+operation which would return a nullptr. Now stealing an array also
+provides the free function reducing one source of memory leaks.
+
+To properly steal memory from VTK-m you do the following:
+```cpp
+  vtkm::cont::ArrayHandle<T> arrayHandle;
+  
+  ...
+  
+  auto* stolen = arrayHandle.StealArray();
+  T* ptr = stolen.first;
+  auto free_function = stolen.second;
+  
+  ...
+
+  free_function(ptr);
+```

vtkm/cont/StorageBasic.h

@@ -11,6 +11,7 @@
 #define vtk_m_cont_StorageBasic_h
 
 #include <vtkm/Assert.h>
+#include <vtkm/Pair.h>
 #include <vtkm/Types.h>
 #include <vtkm/cont/ErrorBadAllocation.h>
 #include <vtkm/cont/ErrorBadValue.h>
@@ -116,14 +117,6 @@ public:
   /// instances that have been stolen (\c StealArray)
   VTKM_CONT bool WillDeallocate() const { return this->DeleteFunction != nullptr; }
 
-  /// \brief Return the free function that will be used to free this memory.
-  ///
-  /// Get the function that VTK-m will call to deallocate the memory. This
-  /// is useful when stealing memory from VTK-m so that you call the correct
-  /// free/delete function when releasing the memory
-  using DeleteFunctionSignature = void (*)(void*);
-  DeleteFunctionSignature GetDeleteFunction() const { return this->DeleteFunction; }
-
   /// \brief Change the Change the pointer that this class is using. Should only be used
   /// by ExecutionArrayInterface sublcasses.
   /// Note: This will release any previous allocated memory!
@@ -197,17 +190,17 @@ public:
 
   VTKM_CONT const ValueType* GetArray() const;
 
-  /// \brief Take the reference away from this object.
+  /// \brief Transfer ownership of the underlying away from this object.
   ///
-  /// This method returns the pointer to the array held by this array. It then
-  /// clears the internal ownership flags, thereby ensuring that the
-  /// Storage will never deallocate the array or be able to reallocate it. This
-  /// is helpful for taking a reference for an array created internally by
+  /// This method returns the pointer and free function to the array held
+  /// by this array. It then clears the internal ownership flags, thereby ensuring
+  /// that the Storage will never deallocate the array or be able to reallocate it.
+  /// This is helpful for taking a reference for an array created internally by
   /// VTK-m and not having to keep a VTK-m object around. Obviously the caller
-  /// becomes responsible for destroying the memory, and should before hand
-  /// grab the correct DeleteFunction by calling \c GetDeleteFunction
+  /// becomes responsible for destroying the memory, and should use the provided
+  /// delete function.
   ///
-  VTKM_CONT ValueType* StealArray();
+  VTKM_CONT vtkm::Pair<ValueType*, void (*)(void*)> StealArray();
 };
 
 } // namespace internal

vtkm/cont/StorageBasic.hxx

@@ -103,10 +103,11 @@ const T* Storage<T, vtkm::cont::StorageTagBasic>::GetArray() const
 }
 
 template <typename T>
-T* Storage<T, vtkm::cont::StorageTagBasic>::StealArray()
+vtkm::Pair<T*, void (*)(void*)> Storage<T, vtkm::cont::StorageTagBasic>::StealArray()
 {
+  vtkm::Pair<T*, void (*)(void*)> result(static_cast<T*>(this->Array), this->DeleteFunction);
   this->DeleteFunction = nullptr;
-  return static_cast<T*>(this->Array);
+  return result;
 }
 
 } // namespace internal

vtkm/cont/testing/UnitTestStorageBasic.cxx

@@ -74,7 +74,8 @@ struct TemplatedTests
     // This call steals the array and prevents deallocation.
     VTKM_TEST_ASSERT(stealMyArray.WillDeallocate() == true,
                      "Array to be stolen needs to be owned by VTK-m");
-    stolenArray = stealMyArray.StealArray();
+    auto stolen = stealMyArray.StealArray();
+    stolenArray = stolen.first;
     VTKM_TEST_ASSERT(stealMyArray.WillDeallocate() == false,
                      "Stolen array should not be owned by VTK-m");
 

vtkm/rendering/MapperGL.cxx

@@ -291,8 +291,8 @@ VTKM_CONT void RenderTriangles(MapperGL& mapper,
 
 
     vtkm::Id vtx_cnt = out_vertices.GetNumberOfValues();
-    vtkm::Float32* v_ptr = out_vertices.GetStorage().StealArray();
-    vtkm::Float32* c_ptr = out_color.GetStorage().StealArray();
+    vtkm::Float32* v_ptr = out_vertices.GetStorage().GetArray();
+    vtkm::Float32* c_ptr = out_color.GetStorage().GetArray();
 
     vtkm::Id floatSz = static_cast<vtkm::Id>(sizeof(vtkm::Float32));
     GLsizeiptr sz = static_cast<GLsizeiptr>(vtx_cnt * floatSz);