From adbbb780fd1a984fecdab13309d1cd7214ef3726 Mon Sep 17 00:00:00 2001 From: Libby Date: Fri, 13 Nov 2026 02:53:48 +0000 Subject: [PATCH] Python: JSONL store writes through commit_store_file (jsonl-tmp-leak-on-replace-failure) (#154) --- packages/cultcache-py/CHANGELOG.md | 1 + packages/cultcache-py/src/cultcache_py/stores.py | 4 +--- packages/cultcache-py/tests/test_store_replace.py | 13 +++++++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/cultcache-py/CHANGELOG.md b/packages/cultcache-py/CHANGELOG.md index c58178cb..c222eb82 100644 --- a/packages/cultcache-py/CHANGELOG.md +++ b/packages/cultcache-py/CHANGELOG.md @@ -9,6 +9,7 @@ ### Changed - `msgpack` is a hard dependency, as `pyproject.toml` declared it. The optional-import fallbacks and the `cultcache-py[msgpack]` extra they named (which never existed) are gone: a missing msgpack is an `ImportError` at import. +- `JsonLinesBackingStore` writes through the same `commit_store_file` as the single-file store: a unique staged file, fsynced before the replace and removed when the replace fails, so a refused replace no longer leaves `.tmp` behind. It staged a fixed `.tmp` without an fsync. - The single-file store's `push`, `push_all` and `delete` take the cross-process lock on `.lock` (flock; LockFileEx over the whole file on Windows) across the read and the replace, waiting up to `lock_timeout` seconds (30 by default, `TimeoutError` after), and exclude the C#, Rust and TypeScript runtimes' writers. The staged file is `.tmp--`, fsynced before the replace, with a directory fsync after it off Windows. A fixed `.tmp` is no longer used. ### Breaking diff --git a/packages/cultcache-py/src/cultcache_py/stores.py b/packages/cultcache-py/src/cultcache_py/stores.py index b950619c..36729d44 100644 --- a/packages/cultcache-py/src/cultcache_py/stores.py +++ b/packages/cultcache-py/src/cultcache_py/stores.py @@ -130,7 +130,6 @@ class JsonLinesBackingStore: def _replace_all(self, envelopes: list[CultCacheEnvelope]) -> None: self.path.parent.mkdir(parents=True, exist_ok=True) - temp = self.path.with_suffix(self.path.suffix + ".tmp") lines = [] for envelope in sorted(envelopes, key=lambda item: (item.type, item.key)): lines.append(json.dumps({ @@ -139,8 +138,7 @@ class JsonLinesBackingStore: "payload": base64.b64encode(envelope.payload).decode("ascii"), "stored_at": envelope.stored_at, }, ensure_ascii=False, sort_keys=True)) - temp.write_text("\n".join(lines) + ("\n" if lines else ""), encoding="utf-8") - _replace_store(temp, self.path) + commit_store_file(self.path, ("\n".join(lines) + ("\n" if lines else "")).encode("utf-8")) class SingleFileMessagePackBackingStore: diff --git a/packages/cultcache-py/tests/test_store_replace.py b/packages/cultcache-py/tests/test_store_replace.py index 120fdc60..af8a2595 100644 --- a/packages/cultcache-py/tests/test_store_replace.py +++ b/packages/cultcache-py/tests/test_store_replace.py @@ -107,6 +107,19 @@ class StoreWritesRouteThroughTheReplace(unittest.TestCase): self.assertEqual([destination for _, destination in seen], [binary, lines]) self.assertTrue(binary.read_bytes() and lines.read_text()) + def test_a_failed_jsonl_replace_leaves_no_staged_file_and_the_old_store(self): + envelope = CultCacheEnvelope(key="k", type="t", payload=b"\x01", stored_at="2026-11-06T00:00:00Z") + with tempfile.TemporaryDirectory() as directory: + lines = Path(directory) / "s.jsonl" + store = stores.JsonLinesBackingStore(lines) + store.push_all([envelope]) + before = lines.read_bytes() + with mock.patch.object(stores, "_replace_store", side_effect=OSError("replace refused")): + with self.assertRaises(OSError): + store.push_all([CultCacheEnvelope(key="k2", type="t", payload=b"\x02", stored_at="2026-11-06T00:00:00Z")]) + self.assertEqual(lines.read_bytes(), before) + self.assertEqual([p.name for p in Path(directory).iterdir() if ".tmp" in p.name], []) + @unittest.skipUnless(sys.platform == "win32", "needs a real Windows") def test_real_kernel_replaces_through_both_primitives(self): kernel = stores.WindowsReplaceKernel()