From 6c41a341e57abd671cc5e9b5db5f2eb8c5f78d85 Mon Sep 17 00:00:00 2001
From: saphid <4596216+saphid@users.noreply.github.com>
Date: Mon, 28 Sep 2026 22:30:08 +1000
Subject: [PATCH] App data: serialise restores of a package with a lock beside
 its data

Two clients restoring the same package could each swap directories and then
delete the other's pre-restore copy. The swap and retention cleanup now run
under flock on .<package>.restore.lock.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 frame/android/app-data.py        | 35 ++++++++++++++++++++++----------
 tests/test_frame_android_data.py | 25 +++++++++++++++++++++++
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/frame/android/app-data.py b/frame/android/app-data.py
index 4b216be..15c740b 100644
--- a/frame/android/app-data.py
+++ b/frame/android/app-data.py
@@ -1,4 +1,5 @@
 """Private-data archives, run under podman unshare on the Frame. Stdlib only."""
+import contextlib
 import json
 import os
 from pathlib import Path, PurePosixPath
@@ -113,21 +114,33 @@ def restore(root, package, instance, input_stream):
                     apply_metadata(target, member)
             for target, member in reversed(directories):
                 apply_metadata(target, member)
-        previous = root / ('.' + package + '.before-restore-' + str(time.time_ns()))
-        source.rename(previous)
-        try:
-            (stage / 'data').rename(source)
-        except BaseException:
-            previous.rename(source)
-            raise
-        # Keep only the newest pre-restore copy of this package's data.
-        for old in root.glob('.' + package + '.before-restore-*'):
-            if old != previous and not old.is_symlink():
-                shutil.rmtree(str(old), ignore_errors=True)
+        with package_lock(root, package):  # another restore of this package must not delete our copy
+            previous = root / ('.' + package + '.before-restore-' + str(time.time_ns()))
+            source.rename(previous)
+            try:
+                (stage / 'data').rename(source)
+            except BaseException:
+                previous.rename(source)
+                raise
+            # Keep only the newest pre-restore copy of this package's data.
+            for old in root.glob('.' + package + '.before-restore-*'):
+                if old != previous and not old.is_symlink():
+                    shutil.rmtree(str(old), ignore_errors=True)
         result['previous'] = str(previous)
         return result
 
 
+@contextlib.contextmanager
+def package_lock(root, package):
+    import fcntl
+    fd = os.open(str(root / ('.' + package + '.restore.lock')), os.O_RDWR | os.O_CREAT | os.O_NOFOLLOW, 0o600)
+    try:
+        fcntl.flock(fd, fcntl.LOCK_EX)
+        yield
+    finally:
+        os.close(fd)  # releases the lock
+
+
 def apply_metadata(path, member):
     os.chown(str(path), member.uid, member.gid)
     os.chmod(str(path), member.mode & 0o777)
diff --git a/tests/test_frame_android_data.py b/tests/test_frame_android_data.py
index 5ecf603..ed84e9c 100644
--- a/tests/test_frame_android_data.py
+++ b/tests/test_frame_android_data.py
@@ -140,6 +140,31 @@ class BackupTests(unittest.TestCase):
             self.assertFalse((source / 'lib').exists() or (source / 'lib').is_symlink())
             self.assertEqual((source / 'files/save-link').read_bytes(), b'save')
 
+    def test_concurrent_restores_of_a_package_are_serialised(self):
+        import fcntl, threading
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            (root / PKG).mkdir()
+            (root / PKG / 'save').write_bytes(b'backup')
+            archive = io.BytesIO()
+            REMOTE['backup'](root, PKG, META['instance'], archive)
+            (root / PKG / 'save').write_bytes(b'current')
+            first = REMOTE['restore'](root, PKG, META['instance'], io.BytesIO(archive.getvalue()))['previous']
+            # Another client's restore is mid-swap: it holds the package lock.
+            fd = os.open(str(root / ('.' + PKG + '.restore.lock')), os.O_RDWR | os.O_CREAT, 0o600)
+            fcntl.flock(fd, fcntl.LOCK_EX)
+            results = []
+            second = threading.Thread(target=lambda: results.append(
+                REMOTE['restore'](root, PKG, META['instance'], io.BytesIO(archive.getvalue()))))
+            second.start()
+            second.join(.5)
+            self.assertTrue(second.is_alive())  # waiting, so it can't swap or delete the other's copy
+            self.assertEqual(sorted(root.glob('.' + PKG + '.before-restore-*')), [Path(first)])
+            os.close(fd)
+            second.join(5)
+            self.assertEqual(sorted(root.glob('.' + PKG + '.before-restore-*')), [Path(results[0]['previous'])])
+            self.assertEqual((root / PKG / 'save').read_bytes(), b'backup')
+
     def test_restore_keeps_only_latest_previous_copy(self):
         with tempfile.TemporaryDirectory() as tmp:
             root = Path(tmp)