mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-08-05 02:45:28 +00:00
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
49 lines
1.8 KiB
Python
49 lines
1.8 KiB
Python
"""Atomic JSON file writes.
|
|
|
|
Use this everywhere a JSON config file is persisted. A plain `open("w") +
|
|
json.dump` truncates the file on first write and only fills it with new
|
|
content afterwards — a kill -9 / power loss / OOM in between produces a
|
|
truncated or empty file. For password DBs (`auth.json`) and live state
|
|
(`sessions.json`, `settings.json`, `integrations.json`, `cookbook_state.json`),
|
|
that's a data-loss event.
|
|
|
|
`atomic_write_json` writes to a sibling tmp file, fsyncs, then `os.replace`s
|
|
into place. On POSIX `os.replace` is atomic on the same filesystem.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import os
|
|
import uuid
|
|
from typing import Any, Optional
|
|
|
|
|
|
def atomic_write_json(path: str, data: Any, *, indent: Optional[int] = None) -> None:
|
|
"""Atomically persist `data` as JSON at `path`.
|
|
|
|
The temp file uses a random suffix so two concurrent writers saving the
|
|
same file don't collide on the rename target. A PID suffix does not do
|
|
this: the PID is constant for the life of a process, so two writers on
|
|
the same path within one process (or one single-process container, where
|
|
the PID never changes at all) still race for the same temp file.
|
|
"""
|
|
os.makedirs(os.path.dirname(path) or ".", exist_ok=True)
|
|
tmp = f"{path}.tmp.{uuid.uuid4().hex}"
|
|
with open(tmp, "w", encoding="utf-8") as f:
|
|
json.dump(data, f, indent=indent)
|
|
f.flush()
|
|
os.fsync(f.fileno())
|
|
os.replace(tmp, path)
|
|
|
|
|
|
def atomic_write_text(path: str, text: str) -> None:
|
|
if not isinstance(text, str):
|
|
raise TypeError("atomic_write_text expects a string")
|
|
os.makedirs(os.path.dirname(path) or ".", exist_ok=True)
|
|
tmp = f"{path}.tmp.{uuid.uuid4().hex}"
|
|
with open(tmp, "w", encoding="utf-8") as f:
|
|
f.write(text)
|
|
f.flush()
|
|
os.fsync(f.fileno())
|
|
os.replace(tmp, path)
|