Commit 674fe1fb authored by Robert Maynard's avatar Robert Maynard
Browse files

StealArray now returns the array and free function as a Pair.

This helps reduces bugs when the callers ask to steal arrays
without getting the free function, or ask for the free function
after stealing the array.
parent 75bf9d36
## `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);
```
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#define vtk_m_cont_StorageBasic_h #define vtk_m_cont_StorageBasic_h
#include <vtkm/Assert.h> #include <vtkm/Assert.h>
#include <vtkm/Pair.h>
#include <vtkm/Types.h> #include <vtkm/Types.h>
#include <vtkm/cont/ErrorBadAllocation.h> #include <vtkm/cont/ErrorBadAllocation.h>
#include <vtkm/cont/ErrorBadValue.h> #include <vtkm/cont/ErrorBadValue.h>
...@@ -116,14 +117,6 @@ public: ...@@ -116,14 +117,6 @@ public:
/// instances that have been stolen (\c StealArray) /// instances that have been stolen (\c StealArray)
VTKM_CONT bool WillDeallocate() const { return this->DeleteFunction != nullptr; } 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 /// \brief Change the Change the pointer that this class is using. Should only be used
/// by ExecutionArrayInterface sublcasses. /// by ExecutionArrayInterface sublcasses.
/// Note: This will release any previous allocated memory! /// Note: This will release any previous allocated memory!
...@@ -197,17 +190,17 @@ public: ...@@ -197,17 +190,17 @@ public:
VTKM_CONT const ValueType* GetArray() const; 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 /// This method returns the pointer and free function to the array held
/// clears the internal ownership flags, thereby ensuring that the /// by this array. It then clears the internal ownership flags, thereby ensuring
/// Storage will never deallocate the array or be able to reallocate it. This /// that the Storage will never deallocate the array or be able to reallocate it.
/// is helpful for taking a reference for an array created internally by /// 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 /// 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 /// becomes responsible for destroying the memory, and should use the provided
/// grab the correct DeleteFunction by calling \c GetDeleteFunction /// delete function.
/// ///
VTKM_CONT ValueType* StealArray(); VTKM_CONT vtkm::Pair<ValueType*, void (*)(void*)> StealArray();
}; };
} // namespace internal } // namespace internal
......
...@@ -103,10 +103,11 @@ const T* Storage<T, vtkm::cont::StorageTagBasic>::GetArray() const ...@@ -103,10 +103,11 @@ const T* Storage<T, vtkm::cont::StorageTagBasic>::GetArray() const
} }
template <typename T> 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; this->DeleteFunction = nullptr;
return static_cast<T*>(this->Array); return result;
} }
} // namespace internal } // namespace internal
......
...@@ -74,7 +74,8 @@ struct TemplatedTests ...@@ -74,7 +74,8 @@ struct TemplatedTests
// This call steals the array and prevents deallocation. // This call steals the array and prevents deallocation.
VTKM_TEST_ASSERT(stealMyArray.WillDeallocate() == true, VTKM_TEST_ASSERT(stealMyArray.WillDeallocate() == true,
"Array to be stolen needs to be owned by VTK-m"); "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, VTKM_TEST_ASSERT(stealMyArray.WillDeallocate() == false,
"Stolen array should not be owned by VTK-m"); "Stolen array should not be owned by VTK-m");
......
...@@ -291,8 +291,8 @@ VTKM_CONT void RenderTriangles(MapperGL& mapper, ...@@ -291,8 +291,8 @@ VTKM_CONT void RenderTriangles(MapperGL& mapper,
vtkm::Id vtx_cnt = out_vertices.GetNumberOfValues(); vtkm::Id vtx_cnt = out_vertices.GetNumberOfValues();
vtkm::Float32* v_ptr = out_vertices.GetStorage().StealArray(); vtkm::Float32* v_ptr = out_vertices.GetStorage().GetArray();
vtkm::Float32* c_ptr = out_color.GetStorage().StealArray(); vtkm::Float32* c_ptr = out_color.GetStorage().GetArray();
vtkm::Id floatSz = static_cast<vtkm::Id>(sizeof(vtkm::Float32)); vtkm::Id floatSz = static_cast<vtkm::Id>(sizeof(vtkm::Float32));
GLsizeiptr sz = static_cast<GLsizeiptr>(vtx_cnt * floatSz); GLsizeiptr sz = static_cast<GLsizeiptr>(vtx_cnt * floatSz);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment