Skip to content

Fence stale FFmpeg session owner publication - #139

Draft
Blackspirits wants to merge 1 commit into
audit/ffmpeg-session-lifecycle-3e4dfrom
audit/ffmpeg-stale-session-publication-af88
Draft

Blackspirits wants to merge 1 commit into
audit/ffmpeg-session-lifecycle-3e4dfrom
audit/ffmpeg-stale-session-publication-af88

Conversation

@Blackspirits

@Blackspirits Blackspirits commented Sep 15, 2026

Copy link
Copy Markdown
Owner

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/subtitleedit main was rechecked at d6917286e355976da953d0ee22208b4bf9461699; its latest change is unrelated and no open upstream PR was found for this boundary.

Finding fixed

CloseFile() increments _loadGeneration and exchanges _session to null before calling Session.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:

  • video decode startup/fallback wrote _owner._decoderName directly;
  • presenter called _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;
  • generation check + frame swap + frame-version increment are ordered under the existing current-frame lock;
  • rejected stale frames are returned to the originating Session's own frame pool;
  • CloseFile() clears the decoder badge with the current frame;
  • every Session→owner decoder-name write and frame publication uses the fenced methods.

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:

  • a stale Session cannot restore its decoder badge;
  • player name returns to plain ffmpeg;
  • a stale frame does not change FrameVersion or CurrentFrameSize;
  • the rejected frame returns to its originating VideoFrameQueue rather than leaking or crossing Sessions.

Scope / branch state

  • dependency/base PR: Harden FFmpeg session startup and teardown lifetime #115
  • base HEAD: af88aa7d7591721f6c509d5432f9a07d532bbeae
  • HEAD: 088211bde3024739667ef55448b483064c14cb90
  • 1 commit
  • 2 files
  • +80 / -8
  • current upstream main checked: d6917286e355976da953d0ee22208b4bf9461699
  • audit draft only; no merge/promotion intended

AI 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 088211bde3024739667ef55448b483064c14cb90

  • SeConv: 488 passed / 2 skipped / 0 failed
  • LibUiLogic: 905 passed / 0 skipped / 0 failed
  • LibSE: 2017 passed / 0 skipped / 0 failed
  • UI: 5236 passed / 9 skipped / 0 failed
  • build: 0 errors / 7 pre-existing warnings
  • retry step skipped
  • failure-artifact upload skipped

Final adversarial review was performed on this exact HEAD after CI; no blocker found.

@Blackspirits Blackspirits left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant