Fence stale FFmpeg session owner publication - #139
Draft
Blackspirits wants to merge 1 commit into
Draft
Blackspirits wants to merge 1 commit into
Blackspirits wants to merge 1 commit into
Conversation
Blackspirits
commented
Sep 15, 2026
Blackspirits
left a comment
Owner
Author
There was a problem hiding this comment.
Final adversarial review on 088211bde3024739667ef55448b483064c14cb90 after CI SubtitleEdit#278: no blocker found. Verified generation invalidation happens before teardown wait, frame/version publication is ordered with the generation check under the current-frame lock, stale frames return to their originating pool, stale decoder writes are rejected, and CloseFile clears owner-visible decoder/frame state. No merge/promotion performed.
This was referenced Sep 15, 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.
Purpose
Independent lifetime follow-up to #115 ("Harden FFmpeg session startup and teardown lifetime").
#115 correctly retains all worker-visible Session resources when a worker does not stop within the teardown timeout, avoiding use-after-free. This audit found a remaining owner-publication race after that timeout: the retained worker can wake later even though CloseFile has already removed its Session and a new load may exist.
Current upstream
SubtitleEdit/subtitleeditmain was rechecked atd6917286e355976da953d0ee22208b4bf9461699; its latest change is unrelated and no open upstream PR was found for this boundary.Finding fixed
CloseFile()increments_loadGenerationand exchanges_sessionto null before callingSession.Dispose().If #115 cannot join a worker in time, Dispose deliberately returns with that Session's resources retained. A surviving video/presenter worker could later still write through its owner reference:
_owner._decoderNamedirectly;_owner.Present(frame, pool)directly.After CloseFile returns, a later LoadFile may already own the player. The stale Session could therefore overwrite the new session's decoder badge or current UI frame despite no longer being the active Session.
Fix
Each Session already captures the owner's monotonic load generation. Owner publication now uses that generation as a fence:
TrySetDecoderNameFromSession()accepts a decoder badge only while the source generation is current;TryPresentFrameFromSession()accepts a frame only while the source generation is current;CloseFile()clears the decoder badge with the current frame;Because CloseFile increments the generation before waiting for Session workers, a worker that survives the timeout is stale before it can publish again. A publication that wins before invalidation is cleaned by CloseFile; one arriving after invalidation is rejected.
Regression coverage
Native-free tests pin that after generation invalidation:
ffmpeg;FrameVersionorCurrentFrameSize;VideoFrameQueuerather than leaking or crossing Sessions.Scope / branch state
af88aa7d7591721f6c509d5432f9a07d532bbeae088211bde3024739667ef55448b483064c14cb90d6917286e355976da953d0ee22208b4bf9461699AI assistance: ChatGPT was used for adversarial post-timeout Session lifetime review, generation-fence modelling, frame ownership analysis and focused native-free regression design.
Final CI
Authoritative run: SubtitleEdit#278 on
088211bde3024739667ef55448b483064c14cb90Final adversarial review was performed on this exact HEAD after CI; no blocker found.