Skip to content

Fix race condition in concurrent subtitle conversion - #17342

Merged
crobibero merged 1 commit into
jellyfin:masterfrom
LTe:fix-subtitle-conversion-race
Jul 20, 2026
Merged

crobibero merged 1 commit into
jellyfin:masterfrom
LTe:fix-subtitle-conversion-race

Conversation

@LTe

@LTe LTe commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Changes

SubtitleEncoder.ConvertSubtitles parsed subtitles via libse's static Subtitle.Parse, which uses a statically cached list of shared SubtitleFormat instances whose per-parse state is mutable — concurrent subtitle requests corrupted each other's output (cues mixed across streams/languages, truncated responses) or failed with a NullReferenceException (HTTP 500). This PR routes the conversion through the injected ISubtitleParser (SubtitleEditParser), which creates a fresh format parser per call. ISubtitleParser.Parse now returns the libse Subtitle directly — its previous SubtitleTrackInfo return value had no callers since the SubtitleEdit writer rework (#16805), and returning the full document keeps writer fidelity (e.g. ASS styling). Adds tests asserting that concurrent conversions match a sequential baseline (fails on master, deterministic) and that sequential conversion is deterministic.

Code assistance

Investigated and implemented with LLM assistance (Claude Code): root cause analysis of the race, the fix, and the accompanying tests. All changes were verified by building and running the test suite, including reproducing the corruption on master and confirming it disappears with the fix.

Issues

#17341

SubtitleEncoder.ConvertSubtitles parsed subtitles with libse's static
Subtitle.Parse, which iterates a statically cached list of shared
SubtitleFormat instances. Format parsers keep mutable per-parse state on
the instance, so concurrent subtitle requests corrupted each other's
output (cues mixed across streams and languages, truncated files) or
failed with NullReferenceException when format detection broke down and
Subtitle.Parse returned null.

Parse through the injected ISubtitleParser instead. SubtitleEditParser
instantiates a fresh format parser per call, so requests no longer share
state. Its Parse method now returns the libse Subtitle directly (the
SubtitleTrackInfo flattening was unused since the SubtitleEdit writer
rework) so the writers keep full fidelity such as ASS styling.
@LTe
LTe marked this pull request as ready for review July 15, 2026 12:57
@sonarqubecloud

Copy link
Copy Markdown

@Shadowghost
Shadowghost requested review from a team and gnattu July 17, 2026 08:35
@Shadowghost Shadowghost moved this from Todo to Review in Jellyfin v12 Jul 17, 2026
@jellyfin-bot

Copy link
Copy Markdown
Contributor

Jellyfin API (v 12.0.0)


Result


No differences. Specifications are equivalent

@crobibero
crobibero merged commit f2b9c68 into jellyfin:master Jul 20, 2026
16 checks passed
@github-project-automation github-project-automation Bot moved this from Review to Done in Jellyfin v12 Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants