mirror of
https://github.com/jellyfin/jellyfin.git
synced 2025-07-09 03:04:24 -04:00
Fix parallel use of not thread-safe SubtitleFormat instance
`SubtitleFormat`'s `LoadSubtitle()` function is not thread-safe. A `SubtitleEditParser` instance's `Parse()` function can be called from multiple threads at the same time. `SubtitleFormat`s are cached in the constructor of each `SubtitleEditParser`, and the same instances are used for each possibly parallel `Parse()` function call, which causes subtitle parse problems. This patch modifies the code, so we only cache the extension -> `SubtitleFormat` type/class mapping and create a new `SubtitleFormat` instance in each `Parse()` call, so no `SubtitleFormat` instance is accessed from multiple threads. Fixes #12113 Kudos for everyone investigating the issue there, most notably @RenV123 for PoC-ing the solution. Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
This commit is contained in:
parent
344cc8b97b
commit
c693da94ce
@ -17,7 +17,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles
|
|||||||
public class SubtitleEditParser : ISubtitleParser
|
public class SubtitleEditParser : ISubtitleParser
|
||||||
{
|
{
|
||||||
private readonly ILogger<SubtitleEditParser> _logger;
|
private readonly ILogger<SubtitleEditParser> _logger;
|
||||||
private readonly Dictionary<string, SubtitleFormat[]> _subtitleFormats;
|
private readonly Dictionary<string, List<Type>> _subtitleFormatTypes;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Initializes a new instance of the <see cref="SubtitleEditParser"/> class.
|
/// Initializes a new instance of the <see cref="SubtitleEditParser"/> class.
|
||||||
@ -26,10 +26,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles
|
|||||||
public SubtitleEditParser(ILogger<SubtitleEditParser> logger)
|
public SubtitleEditParser(ILogger<SubtitleEditParser> logger)
|
||||||
{
|
{
|
||||||
_logger = logger;
|
_logger = logger;
|
||||||
_subtitleFormats = GetSubtitleFormats()
|
_subtitleFormatTypes = GetSubtitleFormatTypes();
|
||||||
.Where(subtitleFormat => !string.IsNullOrEmpty(subtitleFormat.Extension))
|
|
||||||
.GroupBy(subtitleFormat => subtitleFormat.Extension.TrimStart('.'), StringComparer.OrdinalIgnoreCase)
|
|
||||||
.ToDictionary(g => g.Key, g => g.ToArray(), StringComparer.OrdinalIgnoreCase);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
@ -38,13 +35,14 @@ namespace MediaBrowser.MediaEncoding.Subtitles
|
|||||||
var subtitle = new Subtitle();
|
var subtitle = new Subtitle();
|
||||||
var lines = stream.ReadAllLines().ToList();
|
var lines = stream.ReadAllLines().ToList();
|
||||||
|
|
||||||
if (!_subtitleFormats.TryGetValue(fileExtension, out var subtitleFormats))
|
if (!_subtitleFormatTypes.TryGetValue(fileExtension, out var subtitleFormatTypesForExtension))
|
||||||
{
|
{
|
||||||
throw new ArgumentException($"Unsupported file extension: {fileExtension}", nameof(fileExtension));
|
throw new ArgumentException($"Unsupported file extension: {fileExtension}", nameof(fileExtension));
|
||||||
}
|
}
|
||||||
|
|
||||||
foreach (var subtitleFormat in subtitleFormats)
|
foreach (var subtitleFormatType in subtitleFormatTypesForExtension)
|
||||||
{
|
{
|
||||||
|
var subtitleFormat = (SubtitleFormat)Activator.CreateInstance(subtitleFormatType, true)!;
|
||||||
_logger.LogDebug(
|
_logger.LogDebug(
|
||||||
"Trying to parse '{FileExtension}' subtitle using the {SubtitleFormatParser} format parser",
|
"Trying to parse '{FileExtension}' subtitle using the {SubtitleFormatParser} format parser",
|
||||||
fileExtension,
|
fileExtension,
|
||||||
@ -97,11 +95,11 @@ namespace MediaBrowser.MediaEncoding.Subtitles
|
|||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
public bool SupportsFileExtension(string fileExtension)
|
public bool SupportsFileExtension(string fileExtension)
|
||||||
=> _subtitleFormats.ContainsKey(fileExtension);
|
=> _subtitleFormatTypes.ContainsKey(fileExtension);
|
||||||
|
|
||||||
private List<SubtitleFormat> GetSubtitleFormats()
|
private Dictionary<string, List<Type>> GetSubtitleFormatTypes()
|
||||||
{
|
{
|
||||||
var subtitleFormats = new List<SubtitleFormat>();
|
var subtitleFormatTypes = new Dictionary<string, List<Type>>(StringComparer.OrdinalIgnoreCase);
|
||||||
var assembly = typeof(SubtitleFormat).Assembly;
|
var assembly = typeof(SubtitleFormat).Assembly;
|
||||||
|
|
||||||
foreach (var type in assembly.GetTypes())
|
foreach (var type in assembly.GetTypes())
|
||||||
@ -113,9 +111,20 @@ namespace MediaBrowser.MediaEncoding.Subtitles
|
|||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
// It shouldn't be null, but the exception is caught if it is
|
var tempInstance = (SubtitleFormat)Activator.CreateInstance(type, true)!;
|
||||||
var subtitleFormat = (SubtitleFormat)Activator.CreateInstance(type, true)!;
|
var extension = tempInstance.Extension.TrimStart('.');
|
||||||
subtitleFormats.Add(subtitleFormat);
|
if (!string.IsNullOrEmpty(extension))
|
||||||
|
{
|
||||||
|
// Store only the type, we will instantiate from it later
|
||||||
|
if (!subtitleFormatTypes.TryGetValue(extension, out var subtitleFormatTypesForExtension))
|
||||||
|
{
|
||||||
|
subtitleFormatTypes[extension] = [type];
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
subtitleFormatTypesForExtension.Add(type);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
@ -123,7 +132,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return subtitleFormats;
|
return subtitleFormatTypes;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user