fix(registry): persist race on Windows; recover metadata-less done jobs - #301
Merged
Conversation
persist() is called concurrently from the pipeline thread, API threads, and the sweep loop, all sharing one temp path. Two writers could collide, and on Windows os.replace over a file another writer holds open raises an uncaught PermissionError. The write+replace now happens under the existing lock with a unique temp name per call (the _ensure_cached_mp3 pattern), best-effort like the settings store. _recover_done_job required metadata.json, which is written after status flips to done -- a crash in that window left a complete stems dir permanently unrecoverable. Such dirs now recover with a placeholder title, and a minimal metadata.json is written immediately so the next restart takes the normal path (self-healing, not a lasting special case). The stems-present requirement is unchanged. Closes #281 Closes #284
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #281, closes #284. Phase 2 of #273 — plan: #273 (comment) (PR-2C + PR-2E, combined: both are small fixes to the same file).
What
Persist race (#281)
persist()runs concurrently from the pipeline thread, API threads (delete/sections), and the sweep loop — all writing the sameregistry.json.tmp. Interleaved writers could clobber each other, and on Windowsos.replaceover a tmp file another writer holds open raises an uncaughtPermissionError(silently killing a pipeline task via the generic callback). Fix, both belts from the plan:.registry.<uuid>.tmp— the_ensure_cached_mp3pattern),_lock, wrapped best-effort (log-and-return, matching the settings store's documented philosophy), with afinallyunlink so no temp files leak.Crash-window recovery (#284)
_recover_done_jobdemandedmetadata.json, which is written afterstatus=done— a crash in between left a complete stems dir permanently unrecoverable ("re-upload to restore"). Now: recovered with a placeholder title (Recovered track <id6>), and a minimalmetadata.jsonis written immediately so the next restart takes the normal path — self-healing, not a permanent special case. Dirs without stems are still ignored (unchanged rule).Testing
persist()calls: no exceptions, valid JSON, zero stray temp files.test_restore_skips_orphan_without_metadata→test_restore_recovers_orphan_without_metadata(the old assertion was the Jobs finished during a crash window are unrecoverable #284 bug's behavior).🤖 Generated with Claude Code