Skip to content

Commit

Permalink
BUG: Avoid Py_buffer memory leaks, using unique_ptr as scope guard
Browse files Browse the repository at this point in the history
The original implementations of `PyBuffer::_GetImageViewFromArray`,
`PyVectorContainer::_vector_container_from_array`,
`PyVnl::_GetVnlVectorFromArray`, and `PyVnl<TElement>::_GetVnlMatrixFromArray`
might leak memory from `Py_buffer` in case of an exception.

This commit avoids such leaks by replacing the manual
`PyBuffer_Release(&pyBuffer)` calls in those member functions with scope guards.
  • Loading branch information
N-Dekker committed Oct 4, 2024
1 parent d887443 commit aa2e259
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 10 deletions.
8 changes: 4 additions & 4 deletions Modules/Bridge/NumPy/include/itkPyBuffer.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@


#include "itkImportImageContainer.h"
#include <memory> // For unique_ptr.

namespace itk
{
Expand Down Expand Up @@ -81,11 +82,12 @@ PyBuffer<TImage>::_GetImageViewFromArray(PyObject * arr, PyObject * shape, PyObj
return nullptr;
}

[[maybe_unused]] const std::unique_ptr<Py_buffer, decltype(&PyBuffer_Release)> bufferScopeGuard(&pyBuffer,
&PyBuffer_Release);

const Py_ssize_t bufferLength = pyBuffer.len;
void * const buffer = pyBuffer.buf;

PyBuffer_Release(&pyBuffer);

PyObject * const shapeseq = PySequence_Fast(shape, "expected sequence");
const unsigned int dimension = PySequence_Size(shape);

Expand All @@ -109,7 +111,6 @@ PyBuffer<TImage>::_GetImageViewFromArray(PyObject * arr, PyObject * shape, PyObj
if (bufferLength < 0 || static_cast<size_t>(bufferLength) != len)
{
PyErr_SetString(PyExc_RuntimeError, "Size mismatch of image and Buffer.");
PyBuffer_Release(&pyBuffer);
SWIG_Py_DECREF(shapeseq);
return nullptr;
}
Expand Down Expand Up @@ -150,7 +151,6 @@ PyBuffer<TImage>::_GetImageViewFromArray(PyObject * arr, PyObject * shape, PyObj
output->SetNumberOfComponentsPerPixel(numberOfComponents);

SWIG_Py_DECREF(shapeseq);
PyBuffer_Release(&pyBuffer);

return output;
}
Expand Down
6 changes: 4 additions & 2 deletions Modules/Bridge/NumPy/include/itkPyVectorContainer.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#ifndef itkPyVectorContainer_hxx
#define itkPyVectorContainer_hxx

#include <memory> // For unique_ptr.
#include <stdexcept>

namespace itk
Expand Down Expand Up @@ -61,6 +62,9 @@ PyVectorContainer<TElementIdentifier, TElement>::_vector_container_from_array(Py
return nullptr;
}

[[maybe_unused]] const std::unique_ptr<Py_buffer, decltype(&PyBuffer_Release)> bufferScopeGuard(&pyBuffer,
&PyBuffer_Release);

const Py_ssize_t bufferLength = pyBuffer.len;
const void * const buffer = pyBuffer.buf;

Expand All @@ -75,7 +79,6 @@ PyVectorContainer<TElementIdentifier, TElement>::_vector_container_from_array(Py
if (bufferLength < 0 || static_cast<size_t>(bufferLength) != len)
{
PyErr_SetString(PyExc_RuntimeError, "Size mismatch of vector and Buffer.");
PyBuffer_Release(&pyBuffer);
return nullptr;
}
const auto * const data = static_cast<const DataType *>(buffer);
Expand All @@ -85,7 +88,6 @@ PyVectorContainer<TElementIdentifier, TElement>::_vector_container_from_array(Py
{
output->SetElement(ii, data[ii]);
}
PyBuffer_Release(&pyBuffer);

return output;
}
Expand Down
11 changes: 7 additions & 4 deletions Modules/Bridge/NumPy/include/itkPyVnl.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#ifndef itkPyVnl_hxx
#define itkPyVnl_hxx

#include <memory> // For unique_ptr.
#include <stdexcept>

namespace itk
Expand Down Expand Up @@ -60,6 +61,9 @@ PyVnl<TElement>::_GetVnlVectorFromArray(PyObject * arr, PyObject * shape) -> con
return VectorType();
}

[[maybe_unused]] const std::unique_ptr<Py_buffer, decltype(&PyBuffer_Release)> bufferScopeGuard(&pyBuffer,
&PyBuffer_Release);

const Py_ssize_t bufferLength = pyBuffer.len;
const void * const buffer = pyBuffer.buf;

Expand All @@ -74,12 +78,10 @@ PyVnl<TElement>::_GetVnlVectorFromArray(PyObject * arr, PyObject * shape) -> con
if (bufferLength < 0 || static_cast<size_t>(bufferLength) != len)
{
PyErr_SetString(PyExc_RuntimeError, "Size mismatch of vector and Buffer.");
PyBuffer_Release(&pyBuffer);
return VectorType();
}
const auto * const data = static_cast<const DataType *>(buffer);
VectorType output(data, numberOfElements);
PyBuffer_Release(&pyBuffer);

return output;
}
Expand Down Expand Up @@ -127,6 +129,9 @@ PyVnl<TElement>::_GetVnlMatrixFromArray(PyObject * arr, PyObject * shape) -> con
return MatrixType();
}

[[maybe_unused]] const std::unique_ptr<Py_buffer, decltype(&PyBuffer_Release)> bufferScopeGuard(&pyBuffer,
&PyBuffer_Release);

const Py_ssize_t bufferLength = pyBuffer.len;
const void * const buffer = pyBuffer.buf;

Expand All @@ -145,13 +150,11 @@ PyVnl<TElement>::_GetVnlMatrixFromArray(PyObject * arr, PyObject * shape) -> con
if (bufferLength < 0 || static_cast<size_t>(bufferLength) != len)
{
PyErr_SetString(PyExc_RuntimeError, "Size mismatch of matrix and Buffer.");
PyBuffer_Release(&pyBuffer);
return MatrixType();
}

const auto * const data = static_cast<const DataType *>(buffer);
MatrixType output(data, size[0], size[1]);
PyBuffer_Release(&pyBuffer);

return output;
}
Expand Down

0 comments on commit aa2e259

Please sign in to comment.