Fix memory leak in Array move-assignment and destructor #219
Conversation
Array::operator=(Array&&) overwrote m_data without freeing the old buffer, leaking memory on every ObjectPool erase. The destructor also called cudaMalloc instead of cudaFree for CUDA arrays. Co-authored-by: Cursor <[email protected]>
There was a problem hiding this comment.
Pull request overview
Fixes memory management bugs in Array by correctly releasing existing buffers during move-assignment and freeing CUDA-allocated buffers in the destructor.
Changes:
- Replace incorrect
cudaMalloccall inArray::~Array()withcudaFree. - Free the existing
m_databuffer inArray::operator=(Array&&)before taking ownership of the moved-from buffer.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else | ||
| #endif | ||
| std::free(m_data); |
There was a problem hiding this comment.
The destructor unconditionally std::free(m_data) for non-CUDA arrays, but the move-assignment path explicitly avoids freeing MemoryKind::PROXY. If PROXY buffers are not owned (as implied by the new m_kind != MemoryKind::PROXY guard in move-assignment), the destructor should apply the same ownership rule to avoid freeing memory it doesn't own.
| else | |
| #endif | |
| std::free(m_data); | |
| else if (kind() != MemoryKind::PROXY) | |
| std::free(m_data); | |
| #else | |
| if (kind() != MemoryKind::PROXY) | |
| std::free(m_data); | |
| #endif |
| #if TSD_USE_CUDA | ||
| if (kind() == MemoryKind::CUDA) | ||
| cudaMalloc(&m_data, size() * elementSize()); | ||
| cudaFree(m_data); |
There was a problem hiding this comment.
cudaFree returns a status code. If the project has a CUDA error-checking macro/helper (e.g., CUDA_CHECK), use it here (and likewise in move-assignment) so deallocation failures aren’t silently ignored.
|
LGTM, thanks! |
2 participants
Array::operator=(Array&&) overwrote m_data without freeing the old buffer, leaking memory on every ObjectPool erase. The destructor also called cudaMalloc instead of cudaFree for CUDA arrays.