Improved ref keeping of memoryview return values

A single weakref to the memoryview also stores a reference to the data
owning Python object by using one of its methods as a weakref callback.

Author: jim-easterbrook

USAGE.rst

@@ -372,19 +372,36 @@ In some cases this includes writing to the data.
     buf = thumb.copy()
     thumb_im = PIL.Image.open(io.BytesIO(buf.data()))
 
-Since version 0.18.0 python-exiv2 releases the memoryview (when the memory block is resized or deleted) to prevent problems such as segmentation faults:
+Since version 0.18.0 python-exiv2 releases the memoryview_ if the memory is invalidated (e.g. if the memory block is resized) to prevent problems such as segmentation faults:
 
 .. code:: python
 
     >>> buf = exiv2.DataBuf(b'fred')
     >>> data = buf.data()
     >>> print(bytes(data))
     b'fred'
-    >>> del buf
+    >>> buf.resize(128)
     >>> print(bytes(data))
     Traceback (most recent call last):
       File "<stdin>", line 1, in <module>
     ValueError: operation forbidden on released memoryview object
+    >>>
+
+Although memoryview_ objects can be used in a with_ statement this has no benefit with python-exiv2.
+The memory view's ``release`` method does nothing.
+Releasing any associated resources only happens when the memory view is deleted:
+
+.. code:: python
+
+    with buf.data() as data:
+        file.write(data)
+    del data
+
+is equivalent to
+
+.. code:: python
+
+    file.write(buf.data())
 
 Buffer interface
 ----------------
@@ -400,6 +417,8 @@ For example, you could save a photograph's thumbnail in a separate file like thi
     with open('thumbnail.jpg', 'wb') as out_file:
         out_file.write(thumb.copy())
 
+Use of this buffer interface is deprecated (since python-exiv2 v0.18.0) and the ``data()`` methods described above should be used instead.
+
 Image data in memory
 --------------------
 
@@ -410,32 +429,9 @@ The buffered data isn't actually read until ``Image::readMetadata`` is called, s
 When ``Image::writeMetadata`` is called exiv2 allocates a new block of memory to store the modified data.
 The ``Image::io`` method returns an `Exiv2::BasicIo`_ object that provides access to this data.
 
-The ``BasicIo::mmap`` method allows access to the image file data without unnecessary copying.
-However it is rather error prone, crashing your Python program with a segmentation fault if anything goes wrong.
-
-The ``Exiv2::BasicIo`` object must be open when ``mmap()`` is called.
-A Python `context manager`_ can be used to ensure that the ``open()`` and ``mmap()`` calls are paired with ``munmap()`` and ``close()`` calls:
-
-.. code:: python
-
-    from contextlib import contextmanager
-
-    @contextmanager
-    def get_file_data(image):
-        exiv_io = image.io()
-        exiv_io.open()
-        try:
-            yield exiv_io.mmap()
-        finally:
-            exiv_io.munmap()
-            exiv_io.close()
-
-    # after setting some metadata
-    image.writeMetadata()
-    with get_file_data(image) as data:
-        rsp = requests.post(url, files={'file': io.BytesIO(data)})
-
-Since v0.18.0 python-exiv2 has a ``exiv2.BasicIo.data()`` method that is easier to use:
+The ``BasicIo::mmap`` and ``BasicIo::munmap`` methods allows access to the image file data without unnecessary copying.
+However they are rather error prone, crashing your Python program with a segmentation fault if anything goes wrong.
+Since python-exiv2 v0.18.0 it is much easier to use the ``data()`` method:
 
 .. code:: python
 
@@ -444,26 +440,17 @@ Since v0.18.0 python-exiv2 has a ``exiv2.BasicIo.data()`` method that is easier
     exiv_io = image.io()
     rsp = requests.post(url, files={'file': io.BytesIO(exiv_io.data())})
 
-The ``exiv2.BasicIo`` Python type also exposes a `buffer interface`_.
-It allows the ``exiv2.BasicIo`` object to be used anywhere that a `bytes-like object`_ is expected:
-
-.. code:: python
-
-    # after setting some metadata
-    image.writeMetadata()
-    exiv_io = image.io()
-    rsp = requests.post(url, files={'file': io.BytesIO(exiv_io)})
-
-Since python-exiv2 v0.15.0 this buffer can be writeable:
+The ``data()`` method can also return a writeable memoryview_:
 
 .. code:: python
 
     exiv_io = image.io()
-    with memoryview(exiv_io) as data:
-        data[23] = 157      # modifies data buffer
+    data = exiv_io.data(True)
+    data[23] = 157          # modifies data buffer
+    del data                # writes modified data to the buffer
     image.readMetadata()    # reads modified buffer data
 
-The modified data is written back to the file or memory buffer when the memoryview_ is released.
+The modified data is written back to the file or memory buffer when the memoryview_ is deleted.
 
 .. _bytearray:
     https://docs.python.org/3/library/stdtypes.html#bytearray
@@ -513,3 +500,5 @@ The modified data is written back to the file or memory buffer when the memoryvi
     https://docs.python.org/3/library/stdtypes.html#memoryview
 .. _PyPI:
     https://pypi.org/project/exiv2/
+.. _with:
+    https://docs.python.org/3/reference/compound_stmts.html#with

src/interface/basicio.i

@@ -57,7 +57,7 @@ UNIQUE_PTR(Exiv2::BasicIo);
 %thread Exiv2::BasicIo::seek;
 %thread Exiv2::BasicIo::transfer;
 %thread Exiv2::BasicIo::write;
-%thread Exiv2::BasicIo::_release;
+%thread Exiv2::BasicIo::_view_deleted_cb;
 
 // Some calls don't raise exceptions
 %noexception Exiv2::BasicIo::eof;
@@ -158,16 +158,14 @@ RETURN_VIEW(Exiv2::byte* mmap, $1 ? arg1->size() : 0,
 
 // Release memoryviews when some other functions are called
 %typemap(ret, fragment="memoryview_funcs")
-        (int close), (int munmap), (long write), (size_t write),
-        (void _release) %{
+        (int close), (int munmap), (long write), (size_t write) %{
     release_views(self);
 %}
 
 // Add data() method for easy access
 // The callback is used to call munmap when the memoryview is deleted
-RETURN_VIEW_CB(Exiv2::byte* data, $1 ? arg1->size() : 0,
-               _global_writeable ? PyBUF_WRITE : PyBUF_READ,
-               PyObject_GetAttrString(self, "_release"),)
+RETURN_VIEW(Exiv2::byte* data, $1 ? arg1->size() : 0,
+            _global_writeable ? PyBUF_WRITE : PyBUF_READ,)
 %feature("docstring") Exiv2::BasicIo::data
 "Easy access to the IO data.
 
@@ -183,7 +181,7 @@ munmap() and close() are called when the memoryview object is deleted.
         self->open();
         return self->mmap(isWriteable);
     };
-    void _release(PyObject* ref) {
+    void _view_deleted_cb(PyObject* ref) {
         self->munmap();
         self->close();
     };

src/interface/preview.i

@@ -96,10 +96,12 @@ RETURN_VIEW(Exiv2::byte* pData, arg1->size(), PyBUF_READ,
 // Add data() alias of pData()
 RETURN_VIEW(Exiv2::byte* data, arg1->size(), PyBUF_READ,
             Exiv2::PreviewImage::data)
+%noexception Exiv2::PreviewImage::_view_deleted_cb;
 %extend Exiv2::PreviewImage {
-const Exiv2::byte* data() {
-    return $self->pData();
-};
+    const Exiv2::byte* data() {
+        return $self->pData();
+    };
+    void _view_deleted_cb(PyObject* ref) {};
 }
 
 // Deprecate pData() in favour of data() since 2025-07-02

src/interface/shared/buffers.i

@@ -70,71 +70,69 @@
 %}
 %enddef // OUTPUT_BUFFER_RW
 
-// Import exiv2.utilities.view_manager
-%fragment("_import_view_manager_decl", "header") {
-static PyObject* view_manager = NULL;
-}
-%fragment("_import_view_manager", "init",
-          fragment="_import_view_manager_decl") {
-{
-    PyObject* mod = PyImport_ImportModule("exiv2.utilities");
-    if (!mod)
-        return INIT_ERROR_RETURN;
-    view_manager = PyObject_GetAttrString(mod, "view_manager");
-    if (!view_manager) {
-        PyErr_SetString(
-            PyExc_RuntimeError,
-            "Import error: exiv2.utilities.view_manager not found.");
-        return INIT_ERROR_RETURN;
-    }
-}
-}
 
 // Functions to store references to memoryview objects and release them
-%fragment("memoryview_funcs", "header", fragment="private_data",
-          fragment="_import_view_manager") {
-static int store_view(PyObject* py_self, PyObject* view,
-                      PyObject* callback=NULL) {
-    PyObject* view_ref = PyWeakref_NewRef(view, callback);
-    if (!view_ref)
-        return -1;
-    PyObject* marker = private_store_get(py_self, "marker");
-    if (!marker) {
-        // Marker is any weakrefable object.
-        marker = PySet_New(NULL);
-        if (!marker)
+%fragment("memoryview_funcs", "header", fragment="private_data") {
+static int store_view(PyObject* py_self, PyObject* view) {
+    PyObject* view_list = private_store_get(py_self, "view_list");
+    if (!view_list) {
+        view_list = PyList_New(0);
+        if (!view_list)
             return -1;
-        int error = private_store_set(py_self, "marker", marker);
-        Py_DECREF(marker);
+        int error = private_store_set(py_self, "view_list", view_list);
+        Py_DECREF(view_list);
         if (error)
             return -1;
     }
-    PyObject* OK = PyObject_CallMethod(
-        view_manager, "store_view", "(OO)", marker, view_ref);
-    Py_DECREF(view_ref);
-    if (!OK)
+    PyObject* callback = PyObject_GetAttrString(py_self, "_view_deleted_cb");
+    if (!callback)
         return -1;
-    Py_DECREF(OK);
-    return 0;
+    PyObject* view_ref = PyWeakref_NewRef(view, callback);
+    Py_DECREF(callback);
+    if (!view_ref)
+        return -1;
+    int result = PyList_Append(view_list, view_ref);
+    Py_DECREF(view_ref);
+    return result;
 };
 static int release_views(PyObject* py_self) {
-    private_store_del(py_self, "marker");
+    PyObject* view_list = private_store_get(py_self, "view_list");
+    if (!view_list)
+        return 0;
+    PyObject* view_ref = NULL;
+    PyObject* view = NULL;
+    for (Py_ssize_t idx = PyList_Size(view_list); idx > 0; idx--) {
+        view_ref = PyList_GetItem(view_list, idx - 1);
+        view = PyWeakref_GetObject(view_ref);
+        if (view != Py_None)
+            Py_XDECREF(PyObject_CallMethod(view, "release", NULL));
+        PyList_SetSlice(view_list, idx - 1, idx, NULL);
+    }
     return 0;
 };
 }
 
-// Macro to convert byte* return value to memoryview
-// WARNING: return value does not keep a reference to the data it points to
-%define RETURN_VIEW_CB(signature, size_func, flags, callback, doc_method)
+/* Macro to convert byte* (or similar) return value to memoryview
+ *
+ * We can't store a reference to the data owner in the memoryview result
+ * so we store a weak reference to the memoryview in the data owner. To
+ * prevent the data owner being deleted while the memoryview exists we
+ * use a method of the Python data owner as the weakref callback. This
+ * increments the data owner's ref count, preventing it from being deleted,
+ * then decrements it when the memoryview is deleted (and the callback is
+ * called). The callback doesn't have to do anything, but it can be used
+ * for cleanup (e.g. calling BasicIo::munmap).
+ */
+%define RETURN_VIEW(signature, size_func, flags, doc_method)
 %typemap(doctype) signature "memoryview";
-%typemap(out, fragment="memoryview_funcs") (signature) %{
+%typemap(out, fragment="memoryview_funcs") (signature) {
     $result = PyMemoryView_FromMemory((char*)$1, size_func, flags);
     if (!$result)
         SWIG_fail;
     // Store a weak ref to the new memoryview
-    if (store_view(self, $result, callback))
+    if (store_view(self, $result))
         SWIG_fail;
-%}
+}
 #if #doc_method != ""
 %feature("docstring") doc_method
 "Returns a temporary Python memoryview of the object's data.
@@ -143,9 +141,6 @@ WARNING: do not resize or delete the object while using the view.
 
 :rtype: memoryview"
 #endif
-%enddef // RETURN_VIEW_CB
-%define RETURN_VIEW(signature, size_func, flags, doc_method)
-RETURN_VIEW_CB(signature, size_func, flags, NULL, doc_method)
 %enddef // RETURN_VIEW
 
 

src/interface/types.i

@@ -244,6 +244,10 @@ RETURN_VIEW(Exiv2::byte* pData_, arg1->DATABUF_SIZE, PyBUF_WRITE,
             Exiv2::DataBuf::pData_)
 RETURN_VIEW(Exiv2::byte* data, arg1->DATABUF_SIZE, PyBUF_WRITE,
             Exiv2::DataBuf::data)
+%noexception Exiv2::DataBuf::_view_deleted_cb;
+%extend Exiv2::DataBuf {
+    void _view_deleted_cb(PyObject* ref) {};
+}
 
 // Release memoryview when other functions are called
 %typemap(ret, fragment="memoryview_funcs")