fix: use process-unique temp names for session saves - #2376
Conversation
save_to_paths writes session.json and session-history.json as two separate renames, so a crash between them can leave them out of sync, and the old fixed .json.tmp suffix collided when another herdr process or a stalled first save used the same name. Emit a unique temp file beside the target — PID + per-process counter + high-resolution timestamp — so a live save never clobbers a concurrent or abandoned tmp, while the rename stays on the same filesystem and thus atomic.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughSession persistence now creates unique temporary paths for atomic JSON writes. The paths remain beside the target file and include process and sequence identifiers. Tests verify path properties and cleanup after session and history saves. ChangesPersistence temporary paths
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Closing — the maintainer requested no unsolicited fix PRs. These findings are documented in case they're useful for future issue reports. |
save_json_to_path used
target.with_extension("json.tmp")— a fixed name not unique per write. If two processes (e.g. a herdr server and client, or two server instances) save concurrently, they clobber each other's in-progress temp file, and one canrename()a file the other is still writing, producing a truncated/garbled session.json that fails to parse on next load (silent total session loss).Fix: use a process-unique temp file name incorporating PID, an atomic counter, and nanosecond timestamp:
.{file_name}.{pid}.{seq}.{nanos}.tmp. This ensures no two writers collide on the same temp path.Regression tests:
unique_tmp_path_is_unique_per_callandsave_to_paths_leaves_no_tmp_files_behindverify uniqueness and cleanup.cargo fmt --check: cleancargo test --locked persist: 9 passed, 0 failed