Merge pull request #10517 from ThomasWaldmann/robustunpacker-pypy-bufferror

RobustUnpacker: release all memoryviews of the resync buffer before resizing it

Author: ThomasWaldmann

docs/changes.rst

@@ -175,6 +175,8 @@ Fixes:
 - legacy remote: use deques for the RPC call queues (performance fix for
   transfer), #10450
 - check --repair: RobustUnpacker: resync in linear time (performance fix), #10459
+- check --repair: RobustUnpacker: release all memoryviews of the resync buffer before
+  resizing it (BufferError on PyPy nightly)
 - import-tar: convert PAX ACLs to borg's ACL format
 - validate BORG_PACK_MAX_SIZE and BORG_PACK_MAX_COUNT, #10485
 - completion: repository directory completion fixes, #10460

src/borg/archive.py

@@ -2205,21 +2205,25 @@ def process_file(self, *, tarinfo, status, type, tar):
 INCOMPLETE = object()
 
 
-def valid_msgpacked_dict(d):
-    """check if the data <d> looks like a msgpacked item dict
+def valid_msgpacked_dict(d, offset=0):
+    """check if the data <d> at <offset> looks like a msgpacked item dict
 
     That is: a msgpack map whose first key is a msgpack str that looks like an item key. Items are
     packed with sorted keys, so the first key is the alphabetically smallest key of the item. The key
     is not checked against a list of known keys, so items written by a newer borg version (with keys
     this version does not know) are found as well.
+
+    <d> is only indexed, never sliced: so <d> can be a memoryview and no child views of it are created
+    (on PyPy, a child view that was not released keeps the viewed object exported, i.e. not resizable,
+    until the garbage collector collects the view).
     """
     d_len = len(d)
-    if d_len == 0:
+    if d_len <= offset:
         return False
-    if d[0] & 0xF0 == 0x80:  # object is a fixmap (up to 15 elements)
-        offs = 1
-    elif d[0] == 0xDE:  # object is a map16 (up to 2^16-1 elements)
-        offs = 3
+    if d[offset] & 0xF0 == 0x80:  # object is a fixmap (up to 15 elements)
+        offs = offset + 1
+    elif d[offset] == 0xDE:  # object is a map16 (up to 2^16-1 elements)
+        offs = offset + 3
     else:
         # object is not a map (dict)
         # note: we must not have dicts with > 2^16-1 elements
@@ -2237,11 +2241,11 @@ def valid_msgpacked_dict(d):
     else:
         # key is not a (short) str
         return False
-    key = d[key_offs : key_offs + key_len]
-    if key_len == 0 or len(key) < key_len:
+    key_end = key_offs + key_len
+    if key_len == 0 or d_len < key_end:
         return False
     # does the key look like an item key?
-    return all(c in ITEM_KEY_CHARS for c in key)
+    return all(d[i] in ITEM_KEY_CHARS for i in range(key_offs, key_end))
 
 
 class RobustUnpacker:
@@ -2301,6 +2305,10 @@ def _search(self):
         searched_end = max(self._search_offset, data_len - MAX_ITEM_HEADER_LEN)
         pending_offsets = [o for o in self._pending_offsets if data_len - o <= self.MAX_PENDING_ITEM_LEN]
         self._pending_offsets = []
+        # _buffered_data is resized below (and by feed()), which fails while a memoryview of it exists. CPython
+        # frees an unreferenced view at once, PyPy only when the garbage collector runs, so every view of
+        # _buffered_data is released explicitly: data here, the slice in _try_item_start. valid_msgpacked_dict
+        # gets (data, offset) instead of a slice of data, it only indexes data.
         with memoryview(self._buffered_data) as data:
             for offset in pending_offsets:
                 item = self._try_item_start(data, offset)
@@ -2309,7 +2317,7 @@ def _search(self):
                 elif item is not None:
                     return item
             for offset in range(self._search_offset, data_len):
-                if not valid_msgpacked_dict(data[offset:]):
+                if not valid_msgpacked_dict(data, offset):
                     continue
                 item = self._try_item_start(data, offset)
                 if item is INCOMPLETE:

src/borg/testsuite/archive_test.py

@@ -516,6 +516,19 @@ def test_key_length_msgpacked_items():
     assert valid_msgpacked_dict(msgpack.packb(data))
 
 
+def test_valid_msgpacked_dict_offset():
+    packed = msgpack.packb({"path": b"/a/b/c"})
+    data = b"garbage" + packed
+    assert valid_msgpacked_dict(data, len(b"garbage"))
+    assert not valid_msgpacked_dict(data, 0)
+    # the data ends within the key (fixmap header 1 byte, fixstr header 1 byte, key "path" 4 bytes)
+    assert not valid_msgpacked_dict(data[: len(b"garbage") + 1 + 1 + 3], len(b"garbage"))
+    assert not valid_msgpacked_dict(data, len(data))
+    # a memoryview is only indexed, never sliced (see valid_msgpacked_dict)
+    with memoryview(data) as view:
+        assert valid_msgpacked_dict(view, len(b"garbage"))
+
+
 def test_resync_on_item_with_unknown_first_key():
     # after damage, an item whose first key is unknown to this borg version is still found as a resync point.
     item = {"aaa": 1, "path": "foo"}  # "aaa" sorts before all known item keys