`3c29221`

fix(io/atomic): nonce-suffix tmp files to survive PID reuse

Tmp-sibling filenames move from `<path>.tmp.<pid>` to
`<path>.tmp.<pid>.<hex8>`. The nonce is 4 random bytes hex-encoded
so two writers that happen to share a PID (after fork/recycling)
can't collide on the same scratch path. Legacy nonce-less tmps
still get cleaned up by the stale-sweeper — back-compat for a store
that spans a pre- and post-upgrade writer.

Authored by

espadonne 3 weeks ago

SHA: 3c29221dd244679e127a45dab9bbd744e2298baa
Parents: 2c05e5d
Tree: 3a5ecd5

2 changed files

Status	File	+	-
M	`src/dlm/io/atomic.py`	33	11
M	`tests/unit/test_io_atomic.py`	40	0

src/dlm/io/atomic.pymodified

  """Atomic filesystem writes.
  Single entry point for "write to a tmp sibling then `os.replace` onto the
 -final name." Three Phase-0 modules (`dlm.io.text`, `dlm.store.manifest`,
 +final name." Three early modules (`dlm.io.text`, `dlm.store.manifest`,
  `dlm.store.paths`) independently grew this pattern; consolidating here
  gives us one place to add `fsync` / directory-sync semantics later.
 -The tmp file carries the writer's PID so concurrent writers don't stomp
 -each other's scratch files mid-write. After a crash, stale `.tmp.<pid>`
 -files are harmless: they sit next to the real file and are swept up by
 -`cleanup_stale_tmp_files` from within sprints that notice them (e.g.,
 -Sprint 04's store load path).
 +Each tmp file carries the writer's PID plus a random 8-hex-char nonce so
 +concurrent writers don't stomp each other's scratch files — and so PID
 +reuse (after a parent process dies and the kernel recycles the number)
 +can't make a stale tmp match a live peer. After a crash, stale tmps are
 +harmless: they sit next to the real file and are swept up by
 +`cleanup_stale_tmp_files` from the store load path.
  """
  from __future__ import annotations
  import os
 +import re
  from pathlib import Path
  def write_bytes(path: Path, data: bytes) -> None:
      """Atomically replace `path` with `data`.
 -    Writes to `<path>.tmp.<pid>`, then `os.replace()` to the final name.
 -    Atomic on POSIX and on Windows NTFS. Parent directory must exist.
 +    Writes to `<path>.tmp.<pid>.<nonce>`, then `os.replace()` to the
 +    final name. Atomic on POSIX and on Windows NTFS. Parent directory
 +    must exist.
      """
      tmp = _tmp_path(path)
      tmp.write_bytes(data)
  def cleanup_stale_tmp_files(directory: Path) -> list[Path]:
 -    """Remove `*.tmp.<pid>` files in `directory` whose PID is no longer alive.
 +    """Remove tmp-suffix files in `directory` whose PID is no longer alive.
      Returns the list of files actually removed. Safe to call on a missing
      directory (returns empty). Never removes the final target or files
  # --- internals ---------------------------------------------------------------
 +# `<suffix>.tmp.<pid>.<nonce>` where nonce is 8 hex chars. The nonce
 +# makes PID reuse harmless: a recycled PID on a stale tmp can't collide
 +# with a live peer because the nonce differs.
 +_TMP_RE = re.compile(r"\.tmp\.(?P<pid>\d+)\.(?P<nonce>[0-9a-f]{8})$")
++
  def _tmp_path(path: Path) -> Path:
 -    return path.with_suffix(path.suffix + f".tmp.{os.getpid()}")
 +    nonce = os.urandom(4).hex()
 +    return path.with_suffix(path.suffix + f".tmp.{os.getpid()}.{nonce}")
  def _tmp_pid(path: Path) -> int | None:
 -    """Return the PID embedded in a `<name>.tmp.<pid>` filename, or None."""
 +    """Return the PID embedded in a tmp-suffixed filename, or None.
++
 +    Accepts both the nonce-suffixed shape (`<name>.tmp.<pid>.<hex8>`)
 +    and the legacy nonce-less shape (`<name>.tmp.<pid>`) so sweeps on a
 +    store that spans a pre- and post-upgrade writer still clean up
 +    correctly.
 +    """
      name = path.name
 +    m = _TMP_RE.search(name)
 +    if m is not None:
 +        try:
 +            return int(m.group("pid"))
 +        except ValueError:
 +            return None
 +    # Legacy fallback: `<name>.tmp.<pid>` with no nonce.
      marker = ".tmp."
      idx = name.rfind(marker)
      if idx == -1:

tests/unit/test_io_atomic.pymodified

          assert target.read_bytes() == b"hello"
 +class TestNonceSuffix:
 +    """Audit-11 M9: tmp files carry a random nonce so PID reuse can't
 +    collide a stale tmp with a live peer's scratch file."""
++
 +    def test_tmp_path_includes_nonce(self, tmp_path: Path) -> None:
 +        target = tmp_path / "file.bin"
 +        tmp = atomic._tmp_path(target)
 +        # Shape: `file.bin.tmp.<pid>.<8 hex chars>`
 +        parts = tmp.name.rsplit(".", maxsplit=2)
 +        assert len(parts) == 3
 +        assert parts[1].isdigit()  # PID
 +        assert len(parts[2]) == 8
 +        assert all(c in "0123456789abcdef" for c in parts[2])
++
 +    def test_two_calls_yield_different_tmp_names(self, tmp_path: Path) -> None:
 +        """Same PID, two writers, two distinct tmps — nonce distinguishes."""
 +        target = tmp_path / "file.bin"
 +        a = atomic._tmp_path(target)
 +        b = atomic._tmp_path(target)
 +        assert a != b
++
 +    def test_cleanup_recognises_nonce_suffixed_tmp(self, tmp_path: Path) -> None:
 +        live = tmp_path / "real.txt.tmp.1.0a1b2c3d"
 +        dead = tmp_path / "real.txt.tmp.99999999.deadbeef"
 +        live.write_bytes(b"live")
 +        dead.write_bytes(b"dead")
++
 +        def fake_is_alive(pid: int) -> bool:
 +            return pid == 1
++
 +        with patch("dlm.io.atomic._is_alive", side_effect=fake_is_alive):
 +            removed = atomic.cleanup_stale_tmp_files(tmp_path)
++
 +        assert removed == [dead]
 +        assert live.exists()
 +        assert not dead.exists()
++
++
  class TestCleanupStaleTmp:
      def test_removes_only_dead_pid_tmp_files(self, tmp_path: Path) -> None:
 +        """Legacy nonce-less tmps still get cleaned up — back-compat for
 +        sweeps that span a pre-/post-upgrade writer on the same store."""
          live = tmp_path / "real.txt.tmp.1"
          dead = tmp_path / "real.txt.tmp.99999999"
          live.write_bytes(b"live")