fix(core): stop atomic writes from colliding on a constant PID suffix (#5721)

atomic_write_json/atomic_write_text build their temp filename as
"{path}.tmp.{os.getpid()}". os.getpid() is constant for the life of a
process, so it only ever distinguishes concurrent writers that live in
different OS processes. Odysseus runs as a single long-lived process
per container, so two concurrent writers to the same path (e.g. two
request handlers racing a settings save) always compute the identical
temp path. Whichever finishes os.replace() first removes the shared
tmp file out from under the other, which then raises FileNotFoundError
on its own os.replace() instead of landing its write.

Fix: derive the temp suffix from uuid4() instead of the PID, so every
call gets a distinct temp path regardless of process/thread identity.

routes/prefs_routes.py's _save() had an independent, hand-rolled copy
of the exact same PID-suffix logic (not the shared core.atomic_io
helper other routes already use, e.g. routes/auth_routes.py) with the
same bug. Replaced it with a call to atomic_write_json.

Fixes #5596

Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>

tests/test_atomic_io.py

@@ -1,17 +1,19 @@
 """Tests for ``core.atomic_io`` durability and crash-safety behavior.
 
 ``core.atomic_io`` provides ``atomic_write_json`` and ``atomic_write_text``.
-Both write to a sibling ``.tmp.<pid>`` file, ``fsync`` it, then ``os.replace``
-into place so a crash mid-write leaves the previous good copy untouched rather
-than a truncated/empty file.
+Both write to a sibling ``.tmp.<random>`` file, ``fsync`` it, then
+``os.replace`` into place so a crash mid-write leaves the previous good copy
+untouched rather than a truncated/empty file.
 
 These tests cover the happy path (round-trip, indent, parent-dir creation,
-full overwrite, no leftover tmp) and the two failure paths the implementation
-guarantees: the target file is preserved when serialization fails before the
-replace, and when ``os.replace`` itself fails.
+full overwrite, no leftover tmp), the two failure paths the implementation
+guarantees (the target file is preserved when serialization fails before the
+replace, and when ``os.replace`` itself fails), and that two concurrent
+writers to the same path don't collide on the same temp file.
 """
 import importlib.util
 import json
+import threading
 from pathlib import Path
 
 import pytest
@@ -84,6 +86,42 @@ def test_atomic_write_json_leaves_no_tmp_file(tmp_path):
     assert _tmp_siblings(tmp_path, "data.json") == []
 
 
+def test_atomic_write_json_concurrent_writers_do_not_collide(tmp_path):
+    # Both writers run in this same process, so a PID-based tmp suffix is
+    # identical for both: whichever writer finishes first unlinks the tmp
+    # file (via os.replace) out from under the other, which then raises
+    # FileNotFoundError on its own os.replace instead of landing its write.
+    target = tmp_path / "settings.json"
+    orig_dump = json.dump
+    barrier = threading.Barrier(2)
+    errors = []
+
+    def slow_dump(obj, fp, **kwargs):
+        orig_dump(obj, fp, **kwargs)
+        fp.flush()
+        barrier.wait()
+
+    def write(payload):
+        try:
+            atomic_write_json(str(target), payload)
+        except Exception as exc:  # noqa: BLE001 - captured for the assertion below
+            errors.append(exc)
+
+    json.dump = slow_dump
+    try:
+        t1 = threading.Thread(target=write, args=({"writer": "A"},))
+        t2 = threading.Thread(target=write, args=({"writer": "B"},))
+        t1.start()
+        t2.start()
+        t1.join()
+        t2.join()
+    finally:
+        json.dump = orig_dump
+
+    assert errors == []
+    assert json.loads(target.read_text(encoding="utf-8"))["writer"] in ("A", "B")
+
+
 # ---------------------------------------------------------------------------
 # atomic_write_json — failure path: target preserved on serialization error.
 # ---------------------------------------------------------------------------

tests/test_prefs_atomic_write.py

@@ -1,11 +1,12 @@
 import json
 
 import routes.prefs_routes as prefs_routes
+from core import atomic_io
 
 
 def test_save_replaces_prefs_file_atomically(monkeypatch, tmp_path):
     calls = []
-    real_replace = prefs_routes.os.replace
+    real_replace = atomic_io.os.replace
 
     def fake_replace(src, dst):
         calls.append((src, dst))
@@ -13,7 +14,7 @@ def test_save_replaces_prefs_file_atomically(monkeypatch, tmp_path):
 
     prefs_file = tmp_path / "data" / "user_prefs.json"
     monkeypatch.setattr(prefs_routes, "PREFS_FILE", str(prefs_file))
-    monkeypatch.setattr(prefs_routes.os, "replace", fake_replace)
+    monkeypatch.setattr(atomic_io.os, "replace", fake_replace)
 
     prefs_routes._save({"theme": "dark"})