From fea94c9af10a79abe2b681b23e3b629e9875bdb5 Mon Sep 17 00:00:00 2001 From: bigtailfox Date: Tue, 7 Jul 2026 23:32:46 +0800 Subject: [PATCH 1/4] fix: return empty memoryview for uninitialized DATA fields Use a module-level sentinel when Cap'n Proto reports a NULL pointer with zero size so PyBuffer_FillInfo receives a valid address for unset fields. Co-authored-by: Cursor --- capnp/lib/capnp.pyx | 24 ++++++++++++++++++++---- test/test_get_data_view.py | 27 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index 69dc653f3..a8a6b658d 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -54,6 +54,8 @@ _CAPNP_VERSION_MINOR = capnp.CAPNP_VERSION_MINOR _CAPNP_VERSION_MICRO = capnp.CAPNP_VERSION_MICRO _CAPNP_VERSION = capnp.CAPNP_VERSION +cdef char _EMPTY_DATA_VIEW_SENTINEL = 0 + cdef dict _type_registry = {} @@ -1297,6 +1299,9 @@ cdef class _DynamicStructReader: """ cdef C_DynamicValue.Reader val cdef capnp.Data.Reader temp_data + cdef Py_buffer buf + cdef void* data_ptr + cdef size_t data_size try: val = self.thisptr.get(field) @@ -1307,10 +1312,14 @@ cdef class _DynamicStructReader: raise TypeError("Field '{}' is not a DATA field".format(field)) temp_data = val.asData() + data_ptr = temp_data.begin() + data_size = temp_data.size() + + if data_size == 0 and data_ptr == NULL: + data_ptr = &_EMPTY_DATA_VIEW_SENTINEL # Return read-only memoryview - cdef Py_buffer buf - if PyBuffer_FillInfo(&buf, self, temp_data.begin(), temp_data.size(), 1, PyBUF_CONTIG_RO) < 0: + if PyBuffer_FillInfo(&buf, self, data_ptr, data_size, 1, PyBUF_CONTIG_RO) < 0: raise KjException("Failed to create buffer info") return PyMemoryView_FromBuffer(&buf) @@ -1739,6 +1748,9 @@ cdef class _DynamicStructBuilder: """ cdef C_DynamicValue.Builder val cdef capnp.Data.Builder temp_data + cdef Py_buffer buf + cdef void* data_ptr + cdef size_t data_size try: val = self.thisptr.get(field) @@ -1749,10 +1761,14 @@ cdef class _DynamicStructBuilder: raise TypeError("Field '{}' is not a DATA field".format(field)) temp_data = val.asData() + data_ptr = temp_data.begin() + data_size = temp_data.size() + + if data_size == 0 and data_ptr == NULL: + data_ptr = &_EMPTY_DATA_VIEW_SENTINEL # Return writable memoryview - cdef Py_buffer buf - if PyBuffer_FillInfo(&buf, self, temp_data.begin(), temp_data.size(), 0, PyBUF_WRITABLE) < 0: + if PyBuffer_FillInfo(&buf, self, data_ptr, data_size, 0, PyBUF_WRITABLE) < 0: raise KjException("Failed to create buffer info") return PyMemoryView_FromBuffer(&buf) diff --git a/test/test_get_data_view.py b/test/test_get_data_view.py index 22b88cbef..05ad85f74 100644 --- a/test/test_get_data_view.py +++ b/test/test_get_data_view.py @@ -152,6 +152,33 @@ def test_corner_cases_values(all_types): assert msg.get_data_as_view("dataField").tobytes() == binary_data +def test_uninitialized_data_get_view(all_types): + """ + Default DATA fields should expose an empty memoryview instead of failing on a NULL buffer pointer. + """ + builder = all_types.TestAllTypes.new_message() + builder_view = builder.get_data_as_view("dataField") + + assert isinstance(builder_view, memoryview) + assert builder_view.readonly is False + assert len(builder_view) == 0 + assert builder_view.tobytes() == b"" + + reader = all_types.TestAllTypes.new_message().as_reader() + reader_view = reader.get_data_as_view("dataField") + + assert isinstance(reader_view, memoryview) + assert reader_view.readonly is True + assert len(reader_view) == 0 + assert reader_view.tobytes() == b"" + + with pytest.raises(IndexError): + builder_view[0] = 0xFF + + with pytest.raises(ValueError): + builder_view[0:1] = b"\xff" + + def test_error_wrong_type(all_types): """ Test error handling: Calling get_data_as_view on non-Data fields. From aba27bdd921076bd12af45f6a6b845ef3f68f7d5 Mon Sep 17 00:00:00 2001 From: bigtailfox Date: Tue, 7 Jul 2026 23:34:07 +0800 Subject: [PATCH 2/4] fix: release buffer info if memoryview construction fails PyBuffer_FillInfo pins `self` via buf.obj; call PyBuffer_Release on failure so that reference is not leaked. This is safe for sentinel-backed empty views: PyBuffer_Release only decrements buf.obj and does not free buf.buf. Co-authored-by: Cursor --- capnp/lib/capnp.pyx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index a8a6b658d..e60c660f3 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -1321,7 +1321,14 @@ cdef class _DynamicStructReader: # Return read-only memoryview if PyBuffer_FillInfo(&buf, self, data_ptr, data_size, 1, PyBUF_CONTIG_RO) < 0: raise KjException("Failed to create buffer info") - return PyMemoryView_FromBuffer(&buf) + # PyBuffer_FillInfo took a reference to `self` via buf.obj. If building the memoryview + # fails, release it so we don't leak that reference. PyBuffer_Release only decrements + # buf.obj; it does not free buf.buf (including when buf.buf points at the sentinel). + try: + return PyMemoryView_FromBuffer(&buf) + except Exception: + PyBuffer_Release(&buf) + raise cpdef _which_str(self): try: @@ -1770,7 +1777,14 @@ cdef class _DynamicStructBuilder: # Return writable memoryview if PyBuffer_FillInfo(&buf, self, data_ptr, data_size, 0, PyBUF_WRITABLE) < 0: raise KjException("Failed to create buffer info") - return PyMemoryView_FromBuffer(&buf) + # PyBuffer_FillInfo took a reference to `self` via buf.obj. If building the memoryview + # fails, release it so we don't leak that reference. PyBuffer_Release only decrements + # buf.obj; it does not free buf.buf (including when buf.buf points at the sentinel). + try: + return PyMemoryView_FromBuffer(&buf) + except Exception: + PyBuffer_Release(&buf) + raise cpdef as_reader(self): """A method for casting this Builder to a Reader From 802ea01c5b30a466e2f9df9d8315b31bece001f0 Mon Sep 17 00:00:00 2001 From: bigtailfox Date: Tue, 7 Jul 2026 23:34:20 +0800 Subject: [PATCH 3/4] docs: clarify lifetime rules for zero-copy buffer views Document borrowing semantics, mutation hazards, and empty DATA field behavior for get_data_as_view and to_segment_views. Co-authored-by: Cursor --- capnp/lib/capnp.pyx | 47 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index e60c660f3..45fe8ba60 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -1294,8 +1294,15 @@ cdef class _DynamicStructReader: return self.thisptr.hasByField(field.thisptr) cpdef get_data_as_view(self, field): - """ - Efficiently get a read-only memoryview for a DATA field without copying. + """Efficiently get a read-only memoryview for a DATA field without copying. + + .. warning:: + The returned memoryview *borrows* memory owned by this message. It stays valid only + while both the memoryview (and any object derived from it) and this reader are alive; + the reader keeps the underlying message buffer pinned for that duration. Do not let + the message be mutated underneath an outstanding view. + + An unset/empty DATA field yields a valid, zero-length view (it does not raise). """ cdef C_DynamicValue.Reader val cdef capnp.Data.Reader temp_data @@ -1547,8 +1554,21 @@ cdef class _DynamicStructBuilder: cpdef to_segment_views(_DynamicStructBuilder self): """Returns the struct's containing message as zero-copy, read-only segment views. - The returned views borrow memory from the message builder. Do not mutate, reset, or reuse - the builder while the views are still in use. + The returned object is a sequence of read-only buffer-protocol views, one per output + segment. Each view borrows memory owned by the message builder; the segment pointers and + sizes are captured eagerly at call time (a snapshot). + + .. warning:: + The views (and any buffer exported from them, e.g. by ``memoryview()`` or by a + consumer that holds them) keep the builder pinned and remain valid only while no + mutation happens. Do NOT mutate, re-set, reset, or reuse the builder while any view or + exported buffer is still alive -- this includes calls that may allocate (e.g. getting + an unset pointer/struct/list field). Mutating after the snapshot can grow/relocate + segments, leaving the views pointing at stale or truncated data. Sharing the views + across threads or ``await`` points while the builder may change is a data race. + + Lifetime is enforced only by buffer-protocol reference counting (memory is not freed + while a view is held); correctness of the *contents* is the caller's responsibility. :rtype: sequence """ @@ -1747,11 +1767,22 @@ cdef class _DynamicStructBuilder: return _DynamicOrphan()._init(self.thisptr.disown(field), self._parent) cpdef get_data_as_view(self, field): - """ - Efficiently get a writable memoryview for a DATA field without copying. + """Efficiently get a writable memoryview for a DATA field without copying. + + This allows in-place modification of the underlying buffer:: - This allows in-place modification of the underlying buffer: - msg.get_data_as_view('myField')[0] = 0xFF + msg.get_data_as_view('myField')[0] = 0xFF + + .. warning:: + The returned memoryview *borrows* mutable memory owned by this message builder. It is + only valid while both the memoryview (and any object derived from it) and this builder + are alive. Do NOT mutate, re-set, reset, or reuse the builder while a view is + outstanding -- including calls that may allocate (e.g. getting an unset pointer/struct/ + list field), since those can relocate or stale the borrowed memory. Sharing a view + across threads or ``await`` points while the builder may change is a data race. + + An unset/empty DATA field yields a valid but zero-length view (writes are no-ops); to write + into the field, initialize it to the desired size first. """ cdef C_DynamicValue.Builder val cdef capnp.Data.Builder temp_data From ded8d026b5cec3cd5358ca163e268ad965bc079b Mon Sep 17 00:00:00 2001 From: bigtailfox Date: Wed, 8 Jul 2026 00:20:23 +0800 Subject: [PATCH 4/4] fix: pin DATA field views via shared buffer exporter Replace PyMemoryView_FromBuffer with a _BorrowedBufferView holder and PyMemoryView_FromObject so get_data_as_view() correctly pins the struct reader/builder for the memoryview lifetime. Generalize the same exporter for to_segment_views() and add regression tests for packed payload release. Co-authored-by: Cursor --- capnp/lib/capnp.pyx | 159 +++++++++++++++++++------------------ test/test_get_data_view.py | 57 +++++++++++++ 2 files changed, 137 insertions(+), 79 deletions(-) diff --git a/capnp/lib/capnp.pyx b/capnp/lib/capnp.pyx index 45fe8ba60..fd84debc8 100644 --- a/capnp/lib/capnp.pyx +++ b/capnp/lib/capnp.pyx @@ -16,7 +16,7 @@ from capnp.includes.schema_cpp cimport (MessageReader,) from builtins import memoryview as BuiltinsMemoryview from cpython cimport array, Py_buffer, PyObject_CheckBuffer from cpython.buffer cimport PyBUF_SIMPLE, PyBUF_WRITABLE, PyBUF_WRITE, PyBUF_READ, PyBUF_CONTIG_RO, PyBuffer_FillInfo -from cpython.memoryview cimport PyMemoryView_FromMemory, PyMemoryView_FromBuffer +from cpython.memoryview cimport PyMemoryView_FromMemory, PyMemoryView_FromObject from cpython.bytes cimport PyBytes_FromStringAndSize from cpython.exc cimport PyErr_Clear from cpython.pyport cimport PY_SSIZE_T_MAX @@ -1178,20 +1178,24 @@ cdef class _MessageSize: @cython.internal -cdef class _SegmentView: - cdef object _builder +cdef class _BorrowedBufferView: + """Buffer-protocol exporter that pins an owner while a view borrows its memory.""" + cdef object _owner cdef const char* _ptr cdef Py_ssize_t _size + cdef bint _readonly - cdef _init(self, object builder, const char* ptr, Py_ssize_t size): - self._builder = builder - self._ptr = ptr + cdef _init(self, object owner, const void* ptr, Py_ssize_t size, bint readonly): + self._owner = owner + self._ptr = ptr self._size = size + self._readonly = readonly return self def __getbuffer__(self, Py_buffer *buffer, int flags): - if PyBuffer_FillInfo(buffer, self, self._ptr, self._size, 1, flags) < 0: - raise BufferError("Failed to create segment buffer view") + if PyBuffer_FillInfo(buffer, self, self._ptr, self._size, + self._readonly, flags) < 0: + raise BufferError("Failed to create borrowed buffer view") def __releasebuffer__(self, Py_buffer *buffer): pass @@ -1200,7 +1204,56 @@ cdef class _SegmentView: return self._size def __repr__(self): - return '' % self._size + if self._readonly: + return '' % self._size + return '' % self._size + + +cdef inline object _memoryview_borrowing(object owner, void* ptr, Py_ssize_t size, + bint readonly): + cdef _BorrowedBufferView exporter + exporter = _BorrowedBufferView()._init(owner, ptr, size, readonly) + return PyMemoryView_FromObject(exporter) + + +cdef void _data_field_ptr_reader(_DynamicStructReader self, field, + void** data_ptr, size_t* data_size) except *: + cdef C_DynamicValue.Reader val + cdef capnp.Data.Reader temp_data + + try: + val = self.thisptr.get(field) + except KjException as e: + raise e._to_python() from None + + if val.getType() != capnp.TYPE_DATA: + raise TypeError("Field '{}' is not a DATA field".format(field)) + + temp_data = val.asData() + data_ptr[0] = temp_data.begin() + data_size[0] = temp_data.size() + if data_size[0] == 0 and data_ptr[0] == NULL: + data_ptr[0] = &_EMPTY_DATA_VIEW_SENTINEL + + +cdef void _data_field_ptr_builder(_DynamicStructBuilder self, field, + void** data_ptr, size_t* data_size) except *: + cdef C_DynamicValue.Builder val + cdef capnp.Data.Builder temp_data + + try: + val = self.thisptr.get(field) + except KjException as e: + raise e._to_python() from None + + if val.getType() != capnp.TYPE_DATA: + raise TypeError("Field '{}' is not a DATA field".format(field)) + + temp_data = val.asData() + data_ptr[0] = temp_data.begin() + data_size[0] = temp_data.size() + if data_size[0] == 0 and data_ptr[0] == NULL: + data_ptr[0] = &_EMPTY_DATA_VIEW_SENTINEL @cython.internal @@ -1221,10 +1274,11 @@ cdef class _SegmentViews: if word_count > (PY_SSIZE_T_MAX // 8): raise OverflowError("segment is too large to expose as a Python buffer") byte_count = (8 * word_count) - self._views.append(_SegmentView()._init( + self._views.append(_BorrowedBufferView()._init( builder, - segments[i].begin(), - byte_count)) + segments[i].begin(), + byte_count, + True)) return self def __getitem__(self, index): @@ -1297,45 +1351,18 @@ cdef class _DynamicStructReader: """Efficiently get a read-only memoryview for a DATA field without copying. .. warning:: - The returned memoryview *borrows* memory owned by this message. It stays valid only - while both the memoryview (and any object derived from it) and this reader are alive; - the reader keeps the underlying message buffer pinned for that duration. Do not let - the message be mutated underneath an outstanding view. + The returned memoryview *borrows* memory owned by this message. It stays valid while + the memoryview (and any object derived from it) is alive; an internal exporter pins + this reader for that duration. Do not let the message be mutated underneath an + outstanding view. An unset/empty DATA field yields a valid, zero-length view (it does not raise). """ - cdef C_DynamicValue.Reader val - cdef capnp.Data.Reader temp_data - cdef Py_buffer buf cdef void* data_ptr cdef size_t data_size - try: - val = self.thisptr.get(field) - except KjException as e: - raise e._to_python() from None - - if val.getType() != capnp.TYPE_DATA: - raise TypeError("Field '{}' is not a DATA field".format(field)) - - temp_data = val.asData() - data_ptr = temp_data.begin() - data_size = temp_data.size() - - if data_size == 0 and data_ptr == NULL: - data_ptr = &_EMPTY_DATA_VIEW_SENTINEL - - # Return read-only memoryview - if PyBuffer_FillInfo(&buf, self, data_ptr, data_size, 1, PyBUF_CONTIG_RO) < 0: - raise KjException("Failed to create buffer info") - # PyBuffer_FillInfo took a reference to `self` via buf.obj. If building the memoryview - # fails, release it so we don't leak that reference. PyBuffer_Release only decrements - # buf.obj; it does not free buf.buf (including when buf.buf points at the sentinel). - try: - return PyMemoryView_FromBuffer(&buf) - except Exception: - PyBuffer_Release(&buf) - raise + _data_field_ptr_reader(self, field, &data_ptr, &data_size) + return _memoryview_borrowing(self, data_ptr, data_size, True) cpdef _which_str(self): try: @@ -1775,47 +1802,21 @@ cdef class _DynamicStructBuilder: .. warning:: The returned memoryview *borrows* mutable memory owned by this message builder. It is - only valid while both the memoryview (and any object derived from it) and this builder - are alive. Do NOT mutate, re-set, reset, or reuse the builder while a view is - outstanding -- including calls that may allocate (e.g. getting an unset pointer/struct/ - list field), since those can relocate or stale the borrowed memory. Sharing a view - across threads or ``await`` points while the builder may change is a data race. + valid while the memoryview (and any object derived from it) is alive; an internal + exporter pins this builder for that duration. Do NOT mutate, re-set, reset, or reuse the + builder while a view is outstanding -- including calls that may allocate (e.g. getting + an unset pointer/struct/list field), since those can relocate or stale the borrowed + memory. Sharing a view across threads or ``await`` points while the builder may change + is a data race. An unset/empty DATA field yields a valid but zero-length view (writes are no-ops); to write into the field, initialize it to the desired size first. """ - cdef C_DynamicValue.Builder val - cdef capnp.Data.Builder temp_data - cdef Py_buffer buf cdef void* data_ptr cdef size_t data_size - try: - val = self.thisptr.get(field) - except KjException as e: - raise e._to_python() from None - - if val.getType() != capnp.TYPE_DATA: - raise TypeError("Field '{}' is not a DATA field".format(field)) - - temp_data = val.asData() - data_ptr = temp_data.begin() - data_size = temp_data.size() - - if data_size == 0 and data_ptr == NULL: - data_ptr = &_EMPTY_DATA_VIEW_SENTINEL - - # Return writable memoryview - if PyBuffer_FillInfo(&buf, self, data_ptr, data_size, 0, PyBUF_WRITABLE) < 0: - raise KjException("Failed to create buffer info") - # PyBuffer_FillInfo took a reference to `self` via buf.obj. If building the memoryview - # fails, release it so we don't leak that reference. PyBuffer_Release only decrements - # buf.obj; it does not free buf.buf (including when buf.buf points at the sentinel). - try: - return PyMemoryView_FromBuffer(&buf) - except Exception: - PyBuffer_Release(&buf) - raise + _data_field_ptr_builder(self, field, &data_ptr, &data_size) + return _memoryview_borrowing(self, data_ptr, data_size, False) cpdef as_reader(self): """A method for casting this Builder to a Reader diff --git a/test/test_get_data_view.py b/test/test_get_data_view.py index 05ad85f74..776366faf 100644 --- a/test/test_get_data_view.py +++ b/test/test_get_data_view.py @@ -1,4 +1,8 @@ import os +import tempfile +import weakref +from pathlib import Path + import pytest import capnp import sys @@ -233,3 +237,56 @@ def test_view_keeps_message_alive(all_types): gc.collect() assert view.tobytes() == expected_data + + +def test_data_view_exports_through_buffer_exporter(all_types): + """Returned memoryviews should pin an internal exporter, not bare pointers.""" + msg = all_types.TestAllTypes.new_message() + msg.dataField = b"exporter_check" + view = msg.get_data_as_view("dataField") + + assert isinstance(view, memoryview) + assert view.obj is not None + assert len(view.obj) == len(view) + + +def test_data_view_survives_del_builder(all_types): + msg = all_types.TestAllTypes.new_message() + msg.dataField = b"persistence_check" + view = msg.get_data_as_view("dataField") + + del msg + gc.collect() + + assert view.tobytes() == b"persistence_check" + + +def test_data_view_releases_packed_payload(): + schema_text = """ + @0x9d7d4f087df9b6e1; + struct BlobMsg { + data @0 :Data; + } + """ + + class Payload(bytearray): + pass + + td = tempfile.TemporaryDirectory() + path = Path(td.name) / "blob.capnp" + path.write_text(schema_text) + schema = capnp.load(str(path)) + try: + payload = Payload(schema.BlobMsg.new_message(data=b"x" * 4096).to_bytes_packed()) + payload_ref = weakref.ref(payload) + + reader = schema.BlobMsg.from_bytes_packed(payload) + view = reader.get_data_as_view("data") + view.release() + + del view, reader, payload + gc.collect() + + assert payload_ref() is None + finally: + td.cleanup()