Repository navigation
fix: serialize export session operations on a lifecycle lock - #20
Merged
Merged
Conversation
Contributor
Author
|
Checked out PR #20, the branch already contains the fix to serialize export session operations. Fetched check suite logs (no failed workflow runs found for check_suite 103244797119). Ran full CI checks locally: manifest validation, JS syntax, JS tests, print/traceback check, installed test deps, and route tests all pass (43 passed). No local changes needed; branch is clean. No push required. Task list (6/7 completed)
|
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous Review Summary (commit 7623b2d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 7623b2d)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Reviewed by free · Input: 137.4K · Output: 8.2K · Cached: 427.7K |
cchacs
previously approved these changes
Oct 11, 2026
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
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 #18.
An export session's work directory was only guarded by the video-append
append_lock. Mux did not wait for in-flight video/audio uploads and treatedan existing
audio.binas complete; cancellation and the TTL/size sweep couldremove the directory (and mux's own post-success cleanup) while an upload was
still writing; and multiple mux requests could all pass the audio-resolution
step that runs before the session is popped, then race the same work dir.
Reproduced with a stubbed FFmpeg boundary: a mux invoked FFmpeg and returned
FileResponsewhile a video append was still streaming, so FFmpeg read apartial elementary stream and the session was consumed before the upload
finished.
What changed
Every operation that touches a session's work dir now holds that session's
single lifecycle lock (
state["lock"], replacing the append-onlyappend_lock):start once all uploads are finalized — FFmpeg never reads partial media.
a single mux owner: a second concurrent mux waits, then sees the consumed
session and gets a 404. It also keeps cancel/eviction away from the files
while FFmpeg uses them.
_drop_session/_sweep_sessions) take the lockbefore popping and
rmtree, so cleanup never deletes files an operationstill has open. The sweep skips an in-use session rather than blocking.
sessions.get(session_id) is stateunder the lock, so anoperation that lost the race against a consume/evict returns 404 instead of
acting on a stale reference.
removes its work dir instead of leaking it — the response's background
cleanup is only registered on success.
Tests
Six new barrier-based tests, each proven to fail without its guard by running
the suite against a mutated
routes.py:test_mux_waits_for_an_in_flight_upload_before_running_ffmpeg— mux is parkedon the lock while the upload is mid-body; FFmpeg sees the complete stream.
test_mux_waits_for_an_in_flight_audio_upload— a half-writtenaudio.binis never muxed as a complete mix.
test_two_concurrent_muxes_consume_the_session_once— one FFmpeg invocation,statuses
[200, 404], even with both muxes overlapping audio resolution.test_cancel_waits_for_an_active_upload_before_removing_the_work_dir— cancelblocks until the upload finishes, then removes the dir.
test_eviction_skips_a_session_mid_operation— the sweep leaves a lockedsession's files alone and collects it once idle.
test_cancelled_mux_removes_the_work_dir— a cancelled mux leaks nothing.Full suite:
43 passed. Version bumped to0.3.3;CHANGELOG.mdandCLAUDE.mdupdated.big-pickle(free) | 𝕏