aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCody Robibero <cody@robibe.ro>2026-09-06 07:36:13 -0400
committerGitHub <noreply@github.com>2026-09-06 07:36:13 -0400
commit34feb3c04a605437e1b2a2310a24446c39bad476 (patch)
tree08af87b5cdf95d232b68c801efab1e3274752b7b
parent8d108d28707db3b48337659c20762e75cca84fbe (diff)
parent7b638a22bb049196376170306d2f984826e9663a (diff)
Merge pull request #17763 from Shadowghost/fix-refresh-queue-and-directory-cache
Fix items being lost from the refresh queue and bound the directory service caches
-rw-r--r--Emby.Server.Implementations/IO/LibraryMonitor.cs7
-rw-r--r--Emby.Server.Implementations/Library/LibraryManager.cs9
-rw-r--r--Jellyfin.Api/Controllers/LibraryStructureController.cs11
-rw-r--r--MediaBrowser.Controller/LibraryTaskScheduler/LimitedConcurrencyLibraryScheduler.cs152
-rw-r--r--MediaBrowser.Controller/MediaBrowser.Controller.csproj1
-rw-r--r--MediaBrowser.Controller/Providers/DirectoryService.cs111
-rw-r--r--MediaBrowser.Controller/Providers/IDirectoryService.cs13
-rw-r--r--MediaBrowser.Providers/Lyric/LyricManager.cs8
-rw-r--r--MediaBrowser.Providers/Manager/ProviderManager.cs47
-rw-r--r--MediaBrowser.Providers/Subtitles/SubtitleManager.cs7
-rw-r--r--tests/Jellyfin.Controller.Tests/DirectoryServiceTests.cs210
-rw-r--r--tests/Jellyfin.Controller.Tests/LibraryTaskScheduler/LimitedConcurrencyLibrarySchedulerTests.cs216
-rw-r--r--tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs126
-rw-r--r--tests/Jellyfin.Server.Integration.Tests/Controllers/LibraryStructureControllerTests.cs42
14 files changed, 877 insertions, 83 deletions
diff --git a/Emby.Server.Implementations/IO/LibraryMonitor.cs b/Emby.Server.Implementations/IO/LibraryMonitor.cs
index 1bf0f8c76c..0f92e2f03e 100644
--- a/Emby.Server.Implementations/IO/LibraryMonitor.cs
+++ b/Emby.Server.Implementations/IO/LibraryMonitor.cs
@@ -8,6 +8,7 @@ using Emby.Server.Implementations.Library;
using MediaBrowser.Controller.Configuration;
using MediaBrowser.Controller.Entities;
using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Providers;
using MediaBrowser.Model.IO;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
@@ -21,6 +22,7 @@ namespace Emby.Server.Implementations.IO
private readonly ILibraryManager _libraryManager;
private readonly IServerConfigurationManager _configurationManager;
private readonly IFileSystem _fileSystem;
+ private readonly IDirectoryService _directoryService;
private readonly DotIgnoreIgnoreRule _dotIgnoreIgnoreRule;
/// <summary>
@@ -47,6 +49,7 @@ namespace Emby.Server.Implementations.IO
/// <param name="libraryManager">The library manager.</param>
/// <param name="configurationManager">The configuration manager.</param>
/// <param name="fileSystem">The filesystem.</param>
+ /// <param name="directoryService">The directory service.</param>
/// <param name="appLifetime">The <see cref="IHostApplicationLifetime"/>.</param>
/// <param name="dotIgnoreIgnoreRule">The .ignore rule handler.</param>
public LibraryMonitor(
@@ -54,6 +57,7 @@ namespace Emby.Server.Implementations.IO
ILibraryManager libraryManager,
IServerConfigurationManager configurationManager,
IFileSystem fileSystem,
+ IDirectoryService directoryService,
IHostApplicationLifetime appLifetime,
DotIgnoreIgnoreRule dotIgnoreIgnoreRule)
{
@@ -61,6 +65,7 @@ namespace Emby.Server.Implementations.IO
_logger = logger;
_configurationManager = configurationManager;
_fileSystem = fileSystem;
+ _directoryService = directoryService;
_dotIgnoreIgnoreRule = dotIgnoreIgnoreRule;
appLifetime.ApplicationStarted.Register(Start);
@@ -363,6 +368,8 @@ namespace Emby.Server.Implementations.IO
return;
}
+ _directoryService.Invalidate(path);
+
// Ignore certain files, If the parent of an ignored path has a change event, ignore that too
foreach (var i in _tempIgnoredPaths.Keys)
{
diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index 634cb8044c..e6fa94fbef 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -86,6 +86,7 @@ namespace Emby.Server.Implementations.Library
private readonly ExtraResolver _extraResolver;
private readonly IPathManager _pathManager;
private readonly ILocalizationManager _localization;
+ private readonly IDirectoryService _directoryService;
private readonly FastConcurrentLru<Guid, BaseItem> _cache;
private readonly DotIgnoreIgnoreRule _dotIgnoreIgnoreRule;
private readonly IMediaStreamRepository _mediaStreamRepository;
@@ -184,6 +185,7 @@ namespace Emby.Server.Implementations.Library
_pathManager = pathManager;
_dotIgnoreIgnoreRule = dotIgnoreIgnoreRule;
_localization = localization;
+ _directoryService = directoryService;
_extraResolver = new ExtraResolver(loggerFactory.CreateLogger<ExtraResolver>(), namingOptions, directoryService);
_configurationManager.ConfigurationUpdated += ConfigurationUpdated;
@@ -3774,6 +3776,10 @@ namespace Emby.Server.Implementations.Library
AddMediaPathInternal(name, path, false);
}
}
+
+ // The libraries root was listed before this folder existed, so drop that listing:
+ // anything still reading it resolves the library set without the new folder.
+ _directoryService.Invalidate(virtualFolderPath);
}
finally
{
@@ -3956,6 +3962,7 @@ namespace Emby.Server.Implementations.Library
try
{
Directory.Delete(path, true);
+ _directoryService.Invalidate(path);
}
finally
{
@@ -4025,6 +4032,7 @@ namespace Emby.Server.Implementations.Library
if (!string.IsNullOrEmpty(shortcut))
{
_fileSystem.DeleteFile(shortcut);
+ _directoryService.Invalidate(shortcut);
}
var libraryOptions = CollectionFolder.GetLibraryOptions(virtualFolderPath);
@@ -4068,6 +4076,7 @@ namespace Emby.Server.Implementations.Library
}
_fileSystem.CreateShortcut(lnk, _appHost.ReverseVirtualPath(path));
+ _directoryService.Invalidate(lnk);
RemoveContentTypeOverrides(path);
}
diff --git a/Jellyfin.Api/Controllers/LibraryStructureController.cs b/Jellyfin.Api/Controllers/LibraryStructureController.cs
index 5c596c21b9..65bfe25d21 100644
--- a/Jellyfin.Api/Controllers/LibraryStructureController.cs
+++ b/Jellyfin.Api/Controllers/LibraryStructureController.cs
@@ -16,6 +16,7 @@ using MediaBrowser.Controller.Configuration;
using MediaBrowser.Controller.Entities;
using MediaBrowser.Controller.IO;
using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Providers;
using MediaBrowser.Model.Configuration;
using MediaBrowser.Model.Entities;
using Microsoft.AspNetCore.Authorization;
@@ -34,6 +35,7 @@ public class LibraryStructureController : BaseJellyfinApiController
private readonly IServerApplicationPaths _appPaths;
private readonly ILibraryManager _libraryManager;
private readonly ILibraryMonitor _libraryMonitor;
+ private readonly IDirectoryService _directoryService;
/// <summary>
/// Initializes a new instance of the <see cref="LibraryStructureController"/> class.
@@ -41,14 +43,17 @@ public class LibraryStructureController : BaseJellyfinApiController
/// <param name="serverConfigurationManager">Instance of <see cref="IServerConfigurationManager"/> interface.</param>
/// <param name="libraryManager">Instance of <see cref="ILibraryManager"/> interface.</param>
/// <param name="libraryMonitor">Instance of <see cref="ILibraryMonitor"/> interface.</param>
+ /// <param name="directoryService">Instance of <see cref="IDirectoryService"/> interface.</param>
public LibraryStructureController(
IServerConfigurationManager serverConfigurationManager,
ILibraryManager libraryManager,
- ILibraryMonitor libraryMonitor)
+ ILibraryMonitor libraryMonitor,
+ IDirectoryService directoryService)
{
_appPaths = serverConfigurationManager.ApplicationPaths;
_libraryManager = libraryManager;
_libraryMonitor = libraryMonitor;
+ _directoryService = directoryService;
}
/// <summary>
@@ -178,11 +183,11 @@ public class LibraryStructureController : BaseJellyfinApiController
var tempPath = Path.Combine(
rootFolderPath,
Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture));
- Directory.Move(currentPath, tempPath);
+ _directoryService.Move(currentPath, tempPath);
currentPath = tempPath;
}
- Directory.Move(currentPath, newPath);
+ _directoryService.Move(currentPath, newPath);
}
finally
{
diff --git a/MediaBrowser.Controller/LibraryTaskScheduler/LimitedConcurrencyLibraryScheduler.cs b/MediaBrowser.Controller/LibraryTaskScheduler/LimitedConcurrencyLibraryScheduler.cs
index 6da398129a..be75117b6f 100644
--- a/MediaBrowser.Controller/LibraryTaskScheduler/LimitedConcurrencyLibraryScheduler.cs
+++ b/MediaBrowser.Controller/LibraryTaskScheduler/LimitedConcurrencyLibraryScheduler.cs
@@ -17,7 +17,8 @@ namespace MediaBrowser.Controller.LibraryTaskScheduler;
/// </summary>
public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibraryScheduler, IAsyncDisposable
{
- private const int CleanupGracePeriod = 60;
+ private static readonly TimeSpan _cleanupGracePeriod = TimeSpan.FromSeconds(60);
+
private readonly IHostApplicationLifetime _hostApplicationLifetime;
private readonly ILogger<LimitedConcurrencyLibraryScheduler> _logger;
private readonly IServerConfigurationManager _serverConfigurationManager;
@@ -31,6 +32,8 @@ public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibr
private readonly Lock _taskLock = new();
private readonly Channel<TaskQueueItem> _tasks = Channel.CreateUnbounded<TaskQueueItem>();
+ private readonly CancellationTokenSource _disposeTokenSource = new();
+ private readonly TimeSpan _gracePeriod;
private volatile int _workCounter;
private Task? _cleanupTask;
@@ -46,10 +49,34 @@ public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibr
IHostApplicationLifetime hostApplicationLifetime,
ILogger<LimitedConcurrencyLibraryScheduler> logger,
IServerConfigurationManager serverConfigurationManager)
+ : this(hostApplicationLifetime, logger, serverConfigurationManager, _cleanupGracePeriod)
+ {
+ }
+
+ internal LimitedConcurrencyLibraryScheduler(
+ IHostApplicationLifetime hostApplicationLifetime,
+ ILogger<LimitedConcurrencyLibraryScheduler> logger,
+ IServerConfigurationManager serverConfigurationManager,
+ TimeSpan gracePeriod)
{
_hostApplicationLifetime = hostApplicationLifetime;
_logger = logger;
_serverConfigurationManager = serverConfigurationManager;
+ _gracePeriod = gracePeriod;
+ }
+
+ /// <summary>
+ /// Gets the number of runners the scheduler currently keeps alive.
+ /// </summary>
+ internal int ActiveRunnerCount
+ {
+ get
+ {
+ lock (_taskLock)
+ {
+ return _taskRunners.Count;
+ }
+ }
}
private void ScheduleTaskCleanup()
@@ -68,31 +95,65 @@ public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibr
async Task RunCleanupTask()
{
- _logger.LogDebug("Schedule cleanup task in {CleanupGracePerioid} sec.", CleanupGracePeriod);
- await Task.Delay(TimeSpan.FromSeconds(CleanupGracePeriod)).ConfigureAwait(false);
- if (_disposed)
+ while (true)
{
- _logger.LogDebug("Abort cleaning up, already disposed.");
- return;
- }
+ _logger.LogDebug("Schedule cleanup task in {CleanupGracePeriod}.", _gracePeriod);
+ try
+ {
+ await Task.Delay(_gracePeriod, _disposeTokenSource.Token).ConfigureAwait(false);
+ }
+ catch (OperationCanceledException)
+ {
+ _logger.LogDebug("Abort cleaning up, already disposed.");
+ return;
+ }
- lock (_taskLock)
- {
- if (_tasks.Reader.Count > 0 || _workCounter > 0)
+ if (_disposed)
{
- _logger.LogDebug("Delay cleanup task, operations still running.");
- // tasks are still there so its still in use. Reschedule cleanup task.
- // we cannot just exit here and rely on the other invoker because there is a considerable timeframe where it could have already ended.
- _cleanupTask = RunCleanupTask();
+ _logger.LogDebug("Abort cleaning up, already disposed.");
return;
}
+
+ CancellationTokenSource[] runners;
+ lock (_taskLock)
+ {
+ if (_tasks.Reader.Count > 0 || _workCounter > 0)
+ {
+ _logger.LogDebug("Delay cleanup task, operations still running.");
+ // tasks are still there so its still in use. Wait another grace period.
+ // we cannot just exit here and rely on the other invoker because there is a considerable timeframe where it could have already ended.
+ continue;
+ }
+
+ runners = [.. _taskRunners.Keys];
+
+ // Retire the runners before they are told to stop: an operation starting while
+ // they wind down must spawn its own instead of counting these towards the fanout.
+ _taskRunners.Clear();
+
+ // Hand the next operation the ability to schedule a cleanup again. Without this
+ // the very first cleanup would be the only one that ever runs.
+ _cleanupTask = null;
+ }
+
+ _logger.LogDebug("Cleanup runners.");
+ await StopRunners(runners).ConfigureAwait(false);
+ return;
}
+ }
+ }
- _logger.LogDebug("Cleanup runners.");
- foreach (var item in _taskRunners.ToArray())
+ private static async Task StopRunners(CancellationTokenSource[] runners)
+ {
+ foreach (var runner in runners)
+ {
+ try
{
- await item.Key.CancelAsync().ConfigureAwait(false);
- _taskRunners.Remove(item.Key);
+ await runner.CancelAsync().ConfigureAwait(false);
+ }
+ catch (ObjectDisposedException)
+ {
+ // The runner already stopped on its own and disposed its stop source.
}
}
}
@@ -127,12 +188,17 @@ public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibr
{
var stopToken = new CancellationTokenSource();
var combinedSource = CancellationTokenSource.CreateLinkedTokenSource(stopToken.Token, _hostApplicationLifetime.ApplicationStopping);
+
+ // Keyed on its own stop source, because cancelling that is what reaches the linked
+ // source the runner waits on. Cancellation does not travel the other way.
+ // Started without the runner's own token: a task cancelled before it is scheduled
+ // never runs its body, so it would never take itself out of _taskRunners again.
_taskRunners.Add(
- combinedSource,
+ stopToken,
Task.Factory.StartNew(
ItemWorker,
- (combinedSource, stopToken),
- combinedSource.Token,
+ (stopToken, combinedSource),
+ CancellationToken.None,
TaskCreationOptions.PreferFairness,
TaskScheduler.Default));
}
@@ -145,7 +211,7 @@ public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibr
_deadlockDetector.Value = stopToken.TaskStop;
try
{
- while (!stopToken.GlobalStop.Token.IsCancellationRequested)
+ while (!stopToken.GlobalStop.IsCancellationRequested)
{
var item = await _tasks.Reader.ReadAsync(stopToken.GlobalStop.Token).ConfigureAwait(false);
try
@@ -162,15 +228,24 @@ public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibr
}
}
}
- catch (OperationCanceledException) when (stopToken.TaskStop.IsCancellationRequested)
+ catch (OperationCanceledException) when (stopToken.GlobalStop.IsCancellationRequested)
{
// thats how you do it, interupt the waiter thread. There is nothing to do here when it was on purpose.
}
+ catch (ChannelClosedException)
+ {
+ // the scheduler was disposed and will not hand out any more work.
+ }
finally
{
_logger.LogDebug("Cleanup Runner'.");
_deadlockDetector.Value = default!;
- _taskRunners.Remove(stopToken.TaskStop);
+
+ lock (_taskLock)
+ {
+ _taskRunners.Remove(stopToken.TaskStop);
+ }
+
stopToken.GlobalStop.Dispose();
stopToken.TaskStop.Dispose();
}
@@ -195,7 +270,7 @@ public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibr
finally
{
item.Progress.Report(100);
- item.Done.SetResult();
+ item.Done.TrySetResult();
}
}
@@ -285,16 +360,33 @@ public sealed class LimitedConcurrencyLibraryScheduler : ILimitedConcurrencyLibr
_disposed = true;
_tasks.Writer.Complete();
- foreach (var item in _taskRunners)
+
+ // Nobody is left to run these, so release whoever is waiting on them.
+ while (_tasks.Reader.TryRead(out var item))
{
- await item.Key.CancelAsync().ConfigureAwait(false);
+ item.Done.TrySetResult();
}
- if (_cleanupTask is not null)
+ CancellationTokenSource[] runners;
+ Task? cleanupTask;
+ lock (_taskLock)
{
- await _cleanupTask.ConfigureAwait(false);
- _cleanupTask?.Dispose();
+ runners = [.. _taskRunners.Keys];
+ _taskRunners.Clear();
+ cleanupTask = _cleanupTask;
}
+
+ await StopRunners(runners).ConfigureAwait(false);
+
+ // Cuts the grace period short instead of holding up shutdown for the rest of it.
+ await _disposeTokenSource.CancelAsync().ConfigureAwait(false);
+
+ if (cleanupTask is not null)
+ {
+ await cleanupTask.ConfigureAwait(false);
+ }
+
+ _disposeTokenSource.Dispose();
}
private class TaskQueueItem
diff --git a/MediaBrowser.Controller/MediaBrowser.Controller.csproj b/MediaBrowser.Controller/MediaBrowser.Controller.csproj
index 06188ad511..73cdf18e91 100644
--- a/MediaBrowser.Controller/MediaBrowser.Controller.csproj
+++ b/MediaBrowser.Controller/MediaBrowser.Controller.csproj
@@ -18,7 +18,6 @@
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="BitFaster.Caching" />
<PackageReference Include="Microsoft.Extensions.Configuration.Binder" />
</ItemGroup>
diff --git a/MediaBrowser.Controller/Providers/DirectoryService.cs b/MediaBrowser.Controller/Providers/DirectoryService.cs
index 6060d051a5..f8e0bf4ed9 100644
--- a/MediaBrowser.Controller/Providers/DirectoryService.cs
+++ b/MediaBrowser.Controller/Providers/DirectoryService.cs
@@ -5,13 +5,19 @@ using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Linq;
+using System.Threading;
using MediaBrowser.Model.IO;
namespace MediaBrowser.Controller.Providers
{
public class DirectoryService : IDirectoryService
{
- // TODO make static and switch to FastConcurrentLru.
+ // TODO replace with one shared bounded cache.
+ private const int MaxCachedRecords = 100_000;
+ private const int AccessIntervalMs = 1_000;
+ // Timeout cache if no access for 5 minutes.
+ private const int IdleTimeoutMs = 5 * 60 * 1_000;
+
private readonly ConcurrentDictionary<string, FileSystemMetadata[]> _cache = new(StringComparer.Ordinal);
private readonly ConcurrentDictionary<string, FileSystemMetadata> _fileCache = new(StringComparer.Ordinal);
@@ -20,6 +26,12 @@ namespace MediaBrowser.Controller.Providers
private readonly IFileSystem _fileSystem;
+ // ConcurrentDictionary.Count locks the dictionary, so keep an estimated counter.
+ // Concurrent factory runs can overcount and a clear racing an add can undercount,
+ // it only has to be roughly right.
+ private int _recordCount;
+ private long _lastAccess = Environment.TickCount64;
+
public DirectoryService(IFileSystem fileSystem)
{
_fileSystem = fileSystem;
@@ -27,20 +39,26 @@ namespace MediaBrowser.Controller.Providers
public FileSystemMetadata[] GetFileSystemEntries(string path)
{
+ DropCacheIfIdleOrFull();
+
return _cache.GetOrAdd(
path,
- static (p, fileSystem) =>
+ static (p, state) =>
{
+ FileSystemMetadata[] entries;
try
{
- return fileSystem.GetFileSystemEntries(p).ToArray();
+ entries = state.FileSystem.GetFileSystemEntries(p).ToArray();
}
catch (DirectoryNotFoundException)
{
- return [];
+ entries = [];
}
+
+ Interlocked.Add(ref state.Service._recordCount, entries.Length + 1);
+ return entries;
},
- _fileSystem);
+ (FileSystem: _fileSystem, Service: this));
}
public List<FileSystemMetadata> GetDirectories(string path)
@@ -89,13 +107,18 @@ namespace MediaBrowser.Controller.Providers
public FileSystemMetadata? GetFileSystemEntry(string path)
{
+ DropCacheIfIdleOrFull();
+
if (!_fileCache.TryGetValue(path, out var result))
{
var file = _fileSystem.GetFileSystemInfo(path);
if (file?.Exists ?? false)
{
result = file;
- _fileCache.TryAdd(path, result);
+ if (_fileCache.TryAdd(path, result))
+ {
+ Interlocked.Increment(ref _recordCount);
+ }
}
}
@@ -107,32 +130,96 @@ namespace MediaBrowser.Controller.Providers
public IReadOnlyList<string> GetFilePaths(string path, bool clearCache)
{
- if (clearCache)
+ if (clearCache && _filePathCache.TryRemove(path, out var cached))
{
- _filePathCache.TryRemove(path, out _);
+ Interlocked.Add(ref _recordCount, -(cached.Count + 1));
}
+ DropCacheIfIdleOrFull();
+
var filePaths = _filePathCache.GetOrAdd(
path,
- static (p, fileSystem) =>
+ static (p, state) =>
{
+ List<string> filePaths;
try
{
- return fileSystem.GetFilePaths(p).OrderBy(x => x).ToList();
+ filePaths = state.FileSystem.GetFilePaths(p).OrderBy(x => x).ToList();
}
catch (DirectoryNotFoundException)
{
- return [];
+ filePaths = [];
}
+
+ Interlocked.Add(ref state.Service._recordCount, filePaths.Count + 1);
+ return filePaths;
},
- _fileSystem);
+ (FileSystem: _fileSystem, Service: this));
return filePaths;
}
+ public void Invalidate(string path)
+ {
+ Forget(path);
+
+ var parent = Path.GetDirectoryName(path);
+ if (!string.IsNullOrEmpty(parent))
+ {
+ Forget(parent);
+ }
+ }
+
+ public void Move(string source, string destination)
+ {
+ Directory.Move(source, destination);
+
+ Invalidate(source);
+ Invalidate(destination);
+ }
+
public bool IsAccessible(string path)
{
return _fileSystem.GetFileSystemEntryPaths(path).Any();
}
+
+ private void DropCacheIfIdleOrFull()
+ {
+ var nowMs = Environment.TickCount64;
+ var idleMs = nowMs - _lastAccess;
+
+ if (idleMs >= IdleTimeoutMs || _recordCount >= MaxCachedRecords)
+ {
+ _cache.Clear();
+ _fileCache.Clear();
+ _filePathCache.Clear();
+ _recordCount = 0;
+ _lastAccess = nowMs;
+ return;
+ }
+
+ if (idleMs >= AccessIntervalMs)
+ {
+ _lastAccess = nowMs;
+ }
+ }
+
+ private void Forget(string path)
+ {
+ if (_cache.TryRemove(path, out var entries))
+ {
+ Interlocked.Add(ref _recordCount, -(entries.Length + 1));
+ }
+
+ if (_fileCache.TryRemove(path, out _))
+ {
+ Interlocked.Decrement(ref _recordCount);
+ }
+
+ if (_filePathCache.TryRemove(path, out var filePaths))
+ {
+ Interlocked.Add(ref _recordCount, -(filePaths.Count + 1));
+ }
+ }
}
}
diff --git a/MediaBrowser.Controller/Providers/IDirectoryService.cs b/MediaBrowser.Controller/Providers/IDirectoryService.cs
index 8a3fa33da3..3a943d5f0c 100644
--- a/MediaBrowser.Controller/Providers/IDirectoryService.cs
+++ b/MediaBrowser.Controller/Providers/IDirectoryService.cs
@@ -23,6 +23,19 @@ namespace MediaBrowser.Controller.Providers
IReadOnlyList<string> GetFilePaths(string path, bool clearCache);
+ /// <summary>
+ /// Forgets what is cached about a path and about the directory containing it.
+ /// </summary>
+ /// <param name="path">The file or directory path that changed.</param>
+ void Invalidate(string path);
+
+ /// <summary>
+ /// Moves a directory and forgets what is cached about both paths.
+ /// </summary>
+ /// <param name="source">The directory to move.</param>
+ /// <param name="destination">The path to move the directory to.</param>
+ void Move(string source, string destination);
+
bool IsAccessible(string path);
}
}
diff --git a/MediaBrowser.Providers/Lyric/LyricManager.cs b/MediaBrowser.Providers/Lyric/LyricManager.cs
index af31e373ef..a19262c3a7 100644
--- a/MediaBrowser.Providers/Lyric/LyricManager.cs
+++ b/MediaBrowser.Providers/Lyric/LyricManager.cs
@@ -32,6 +32,7 @@ public class LyricManager : ILyricManager
private readonly IFileSystem _fileSystem;
private readonly ILibraryMonitor _libraryMonitor;
private readonly IMediaSourceManager _mediaSourceManager;
+ private readonly IDirectoryService _directoryService;
private readonly ILyricProvider[] _lyricProviders;
private readonly ILyricParser[] _lyricParsers;
@@ -43,6 +44,7 @@ public class LyricManager : ILyricManager
/// <param name="fileSystem">Instance of the <see cref="IFileSystem"/> interface.</param>
/// <param name="libraryMonitor">Instance of the <see cref="ILibraryMonitor"/> interface.</param>
/// <param name="mediaSourceManager">Instance of the <see cref="IMediaSourceManager"/> interface.</param>
+ /// <param name="directoryService">Instance of the <see cref="IDirectoryService"/> interface.</param>
/// <param name="lyricProviders">The list of <see cref="ILyricProvider"/>.</param>
/// <param name="lyricParsers">The list of <see cref="ILyricParser"/>.</param>
public LyricManager(
@@ -50,6 +52,7 @@ public class LyricManager : ILyricManager
IFileSystem fileSystem,
ILibraryMonitor libraryMonitor,
IMediaSourceManager mediaSourceManager,
+ IDirectoryService directoryService,
IEnumerable<ILyricProvider> lyricProviders,
IEnumerable<ILyricParser> lyricParsers)
{
@@ -57,6 +60,7 @@ public class LyricManager : ILyricManager
_fileSystem = fileSystem;
_libraryMonitor = libraryMonitor;
_mediaSourceManager = mediaSourceManager;
+ _directoryService = directoryService;
_lyricProviders = lyricProviders
.OrderBy(i => i is IHasOrder hasOrder ? hasOrder.Order : 0)
.ToArray();
@@ -250,6 +254,8 @@ public class LyricManager : ILyricManager
{
_libraryMonitor.ReportFileSystemChangeComplete(path, false);
}
+
+ _directoryService.Invalidate(path);
}
return audio.RefreshMetadata(CancellationToken.None);
@@ -446,6 +452,8 @@ public class LyricManager : ILyricManager
await stream.CopyToAsync(fs).ConfigureAwait(false);
}
+ _directoryService.Invalidate(savePath);
+
return;
}
catch (Exception ex)
diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs
index fbd9e5435e..e7b15305b3 100644
--- a/MediaBrowser.Providers/Manager/ProviderManager.cs
+++ b/MediaBrowser.Providers/Manager/ProviderManager.cs
@@ -1143,16 +1143,21 @@ namespace MediaBrowser.Providers.Manager
return;
}
- _refreshQueue.Enqueue((itemId, options), priority);
-
+ // PriorityQueue is not thread safe and the processor dequeues concurrently, so every
+ // touch of the queue takes the lock.
lock (_refreshQueueLock)
{
- if (!_isProcessingRefreshQueue)
+ _refreshQueue.Enqueue((itemId, options), priority);
+
+ if (_isProcessingRefreshQueue)
{
- _isProcessingRefreshQueue = true;
- Task.Run(StartProcessingRefreshQueue);
+ return;
}
+
+ _isProcessingRefreshQueue = true;
}
+
+ Task.Run(StartProcessingRefreshQueue);
}
private async Task StartProcessingRefreshQueue()
@@ -1161,17 +1166,33 @@ namespace MediaBrowser.Providers.Manager
if (_disposed)
{
+ lock (_refreshQueueLock)
+ {
+ _isProcessingRefreshQueue = false;
+ }
+
return;
}
var cancellationToken = _disposeCancellationTokenSource.Token;
libraryManager.ClearIgnoreRuleCache();
- while (_refreshQueue.TryDequeue(out var refreshItem, out _))
+
+ while (true)
{
- if (_disposed)
+ (Guid ItemId, MetadataRefreshOptions RefreshOptions) refreshItem;
+
+ // Dequeueing and standing down happen under one lock, otherwise a refresh queued
+ // just after the queue ran dry would see a processor that has already stopped.
+ lock (_refreshQueueLock)
{
- return;
+ if (_disposed
+ || cancellationToken.IsCancellationRequested
+ || !_refreshQueue.TryDequeue(out refreshItem, out _))
+ {
+ _isProcessingRefreshQueue = false;
+ break;
+ }
}
try
@@ -1188,19 +1209,21 @@ namespace MediaBrowser.Providers.Manager
await task.ConfigureAwait(false);
}
- catch (OperationCanceledException)
+ catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
- break;
+ // Shutting down: the next pass sees the token and stands the processor down.
+ continue;
}
catch (Exception ex)
{
+ // Includes a provider that cancelled for its own reasons, such as an HTTP
+ // timeout, which must not stop the queue draining.
_logger.LogError(ex, "Error refreshing item");
}
}
- lock (_refreshQueueLock)
+ if (!_disposed)
{
- _isProcessingRefreshQueue = false;
libraryManager.ClearIgnoreRuleCache();
}
}
diff --git a/MediaBrowser.Providers/Subtitles/SubtitleManager.cs b/MediaBrowser.Providers/Subtitles/SubtitleManager.cs
index c3458d4b2a..cd9dda21a0 100644
--- a/MediaBrowser.Providers/Subtitles/SubtitleManager.cs
+++ b/MediaBrowser.Providers/Subtitles/SubtitleManager.cs
@@ -33,6 +33,7 @@ namespace MediaBrowser.Providers.Subtitles
private readonly ILibraryMonitor _monitor;
private readonly IMediaSourceManager _mediaSourceManager;
private readonly ILocalizationManager _localization;
+ private readonly IDirectoryService _directoryService;
private readonly HashSet<string> _allowedSubtitleFormats;
private readonly ISubtitleProvider[] _subtitleProviders;
@@ -43,6 +44,7 @@ namespace MediaBrowser.Providers.Subtitles
ILibraryMonitor monitor,
IMediaSourceManager mediaSourceManager,
ILocalizationManager localizationManager,
+ IDirectoryService directoryService,
IEnumerable<ISubtitleProvider> subtitleProviders,
NamingOptions namingOptions)
{
@@ -51,6 +53,7 @@ namespace MediaBrowser.Providers.Subtitles
_monitor = monitor;
_mediaSourceManager = mediaSourceManager;
_localization = localizationManager;
+ _directoryService = directoryService;
_subtitleProviders = subtitleProviders
.OrderBy(i => i is IHasOrder hasOrder ? hasOrder.Order : 0)
.ToArray();
@@ -281,6 +284,8 @@ namespace MediaBrowser.Providers.Subtitles
await stream.CopyToAsync(fs).ConfigureAwait(false);
}
+ _directoryService.Invalidate(path);
+
return;
}
else
@@ -395,6 +400,8 @@ namespace MediaBrowser.Providers.Subtitles
_monitor.ReportFileSystemChangeComplete(path, false);
}
+ _directoryService.Invalidate(path);
+
return item.RefreshMetadata(CancellationToken.None);
}
diff --git a/tests/Jellyfin.Controller.Tests/DirectoryServiceTests.cs b/tests/Jellyfin.Controller.Tests/DirectoryServiceTests.cs
index 1f59908a86..e57fbfe473 100644
--- a/tests/Jellyfin.Controller.Tests/DirectoryServiceTests.cs
+++ b/tests/Jellyfin.Controller.Tests/DirectoryServiceTests.cs
@@ -1,3 +1,5 @@
+using System.Globalization;
+using System.IO;
using System.Linq;
using MediaBrowser.Controller.Providers;
using MediaBrowser.Model.IO;
@@ -8,29 +10,31 @@ namespace Jellyfin.Controller.Tests
{
public class DirectoryServiceTests
{
- private const string LowerCasePath = "/music/someartist";
- private const string UpperCasePath = "/music/SOMEARTIST";
+ // Path.GetDirectoryName, which Invalidate uses to find the parent, normalizes the
+ // separators, so cache keys only match the parent it returns when they use the platform's.
+ private static readonly string _lowerCasePath = LocalPath("/music/someartist");
+ private static readonly string _upperCasePath = LocalPath("/music/SOMEARTIST");
private static readonly FileSystemMetadata[] _lowerCaseFileSystemMetadata =
{
new()
{
- FullName = LowerCasePath + "/Artwork",
+ FullName = Path.Combine(_lowerCasePath, "Artwork"),
IsDirectory = true
},
new()
{
- FullName = LowerCasePath + "/Some Other Folder",
+ FullName = Path.Combine(_lowerCasePath, "Some Other Folder"),
IsDirectory = true
},
new()
{
- FullName = LowerCasePath + "/Song 2.mp3",
+ FullName = Path.Combine(_lowerCasePath, "Song 2.mp3"),
IsDirectory = false
},
new()
{
- FullName = LowerCasePath + "/Song 3.mp3",
+ FullName = Path.Combine(_lowerCasePath, "Song 3.mp3"),
IsDirectory = false
}
};
@@ -39,12 +43,12 @@ namespace Jellyfin.Controller.Tests
{
new()
{
- FullName = UpperCasePath + "/Lyrics",
+ FullName = Path.Combine(_upperCasePath, "Lyrics"),
IsDirectory = true
},
new()
{
- FullName = UpperCasePath + "/Song 1.mp3",
+ FullName = Path.Combine(_upperCasePath, "Song 1.mp3"),
IsDirectory = false
}
};
@@ -53,12 +57,12 @@ namespace Jellyfin.Controller.Tests
public void GetFileSystemEntries_GivenPathsWithDifferentCasing_CachesAll()
{
var fileSystemMock = new Mock<IFileSystem>();
- fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == UpperCasePath), false)).Returns(_upperCaseFileSystemMetadata);
- fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == LowerCasePath), false)).Returns(_lowerCaseFileSystemMetadata);
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == _upperCasePath), false)).Returns(_upperCaseFileSystemMetadata);
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == _lowerCasePath), false)).Returns(_lowerCaseFileSystemMetadata);
var directoryService = new DirectoryService(fileSystemMock.Object);
- var upperCaseResult = directoryService.GetFileSystemEntries(UpperCasePath);
- var lowerCaseResult = directoryService.GetFileSystemEntries(LowerCasePath);
+ var upperCaseResult = directoryService.GetFileSystemEntries(_upperCasePath);
+ var lowerCaseResult = directoryService.GetFileSystemEntries(_lowerCasePath);
Assert.Equal(_upperCaseFileSystemMetadata, upperCaseResult);
Assert.Equal(_lowerCaseFileSystemMetadata, lowerCaseResult);
@@ -68,12 +72,12 @@ namespace Jellyfin.Controller.Tests
public void GetFiles_GivenPathsWithDifferentCasing_ReturnsCorrectFiles()
{
var fileSystemMock = new Mock<IFileSystem>();
- fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == UpperCasePath), false)).Returns(_upperCaseFileSystemMetadata);
- fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == LowerCasePath), false)).Returns(_lowerCaseFileSystemMetadata);
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == _upperCasePath), false)).Returns(_upperCaseFileSystemMetadata);
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == _lowerCasePath), false)).Returns(_lowerCaseFileSystemMetadata);
var directoryService = new DirectoryService(fileSystemMock.Object);
- var upperCaseResult = directoryService.GetFiles(UpperCasePath);
- var lowerCaseResult = directoryService.GetFiles(LowerCasePath);
+ var upperCaseResult = directoryService.GetFiles(_upperCasePath);
+ var lowerCaseResult = directoryService.GetFiles(_lowerCasePath);
Assert.Equal(_upperCaseFileSystemMetadata.Where(f => !f.IsDirectory), upperCaseResult);
Assert.Equal(_lowerCaseFileSystemMetadata.Where(f => !f.IsDirectory), lowerCaseResult);
@@ -83,12 +87,12 @@ namespace Jellyfin.Controller.Tests
public void GetDirectories_GivenPathsWithDifferentCasing_ReturnsCorrectDirectories()
{
var fileSystemMock = new Mock<IFileSystem>();
- fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == UpperCasePath), false)).Returns(_upperCaseFileSystemMetadata);
- fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == LowerCasePath), false)).Returns(_lowerCaseFileSystemMetadata);
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == _upperCasePath), false)).Returns(_upperCaseFileSystemMetadata);
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(It.Is<string>(x => x == _lowerCasePath), false)).Returns(_lowerCaseFileSystemMetadata);
var directoryService = new DirectoryService(fileSystemMock.Object);
- var upperCaseResult = directoryService.GetDirectories(UpperCasePath);
- var lowerCaseResult = directoryService.GetDirectories(LowerCasePath);
+ var upperCaseResult = directoryService.GetDirectories(_upperCasePath);
+ var lowerCaseResult = directoryService.GetDirectories(_lowerCasePath);
Assert.Equal(_upperCaseFileSystemMetadata.Where(f => f.IsDirectory), upperCaseResult);
Assert.Equal(_lowerCaseFileSystemMetadata.Where(f => f.IsDirectory), lowerCaseResult);
@@ -248,5 +252,171 @@ namespace Jellyfin.Controller.Tests
Assert.Equal(cachedPaths, result);
Assert.Equal(newPaths, secondResult);
}
+
+ [Fact]
+ public void GetFileSystemEntries_RepeatedPath_ReadsTheFileSystemOnce()
+ {
+ var fileSystemMock = new Mock<IFileSystem>(MockBehavior.Strict);
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(_lowerCasePath))
+ .Returns(_lowerCaseFileSystemMetadata);
+
+ var directoryService = new DirectoryService(fileSystemMock.Object);
+
+ directoryService.GetFileSystemEntries(_lowerCasePath);
+ directoryService.GetFileSystemEntries(_lowerCasePath);
+
+ fileSystemMock.Verify(f => f.GetFileSystemEntries(_lowerCasePath), Times.Once);
+ }
+
+ [Fact]
+ public void Invalidate_GivenADirectory_DropsBothTheListingAndTheFilePaths()
+ {
+ var fileSystemMock = new Mock<IFileSystem>();
+ fileSystemMock.SetupSequence(f => f.GetFileSystemEntries(_lowerCasePath))
+ .Returns(_lowerCaseFileSystemMetadata)
+ .Returns(_upperCaseFileSystemMetadata);
+ fileSystemMock.SetupSequence(f => f.GetFilePaths(_lowerCasePath, false))
+ .Returns(new[] { Path.Combine(_lowerCasePath, "Song 2.mp3") })
+ .Returns(new[] { Path.Combine(_lowerCasePath, "Song 2.mp3"), Path.Combine(_lowerCasePath, "Song 2.srt") });
+
+ var directoryService = new DirectoryService(fileSystemMock.Object);
+ directoryService.GetFileSystemEntries(_lowerCasePath);
+ directoryService.GetFilePaths(_lowerCasePath);
+
+ directoryService.Invalidate(_lowerCasePath);
+
+ Assert.Equal(_upperCaseFileSystemMetadata, directoryService.GetFileSystemEntries(_lowerCasePath));
+ Assert.Equal(2, directoryService.GetFilePaths(_lowerCasePath).Count);
+ }
+
+ [Fact]
+ public void Invalidate_GivenAFile_DropsTheListingOfTheDirectoryHoldingIt()
+ {
+ var newFile = Path.Combine(_lowerCasePath, "Song 2.srt");
+
+ var fileSystemMock = new Mock<IFileSystem>();
+ fileSystemMock.SetupSequence(f => f.GetFileSystemEntries(_lowerCasePath))
+ .Returns(_lowerCaseFileSystemMetadata)
+ .Returns(_upperCaseFileSystemMetadata);
+
+ var directoryService = new DirectoryService(fileSystemMock.Object);
+ directoryService.GetFileSystemEntries(_lowerCasePath);
+
+ directoryService.Invalidate(newFile);
+
+ Assert.Equal(_upperCaseFileSystemMetadata, directoryService.GetFileSystemEntries(_lowerCasePath));
+ }
+
+ [Fact]
+ public void GetFilePaths_ClearingTheCache_KeepsTheParentDirectory()
+ {
+ var parentPath = LocalPath("/music");
+
+ var fileSystemMock = new Mock<IFileSystem>();
+ fileSystemMock.Setup(f => f.GetFilePaths(_lowerCasePath))
+ .Returns(new[] { Path.Combine(_lowerCasePath, "Song 2.mp3") });
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(parentPath))
+ .Returns(_lowerCaseFileSystemMetadata);
+
+ var directoryService = new DirectoryService(fileSystemMock.Object);
+ directoryService.GetFileSystemEntries(parentPath);
+
+ directoryService.GetFilePaths(_lowerCasePath, true);
+
+ directoryService.GetFileSystemEntries(parentPath);
+ fileSystemMock.Verify(f => f.GetFileSystemEntries(parentPath), Times.Once);
+ }
+
+ [Fact]
+ public void GetFileSystemEntries_MoreRecordsThanTheCeiling_DropsCache()
+ {
+ // Charged by the files in a listing, not the number of listings, so a few big folders
+ // reach the limit where a lot of small ones would not.
+ const int FolderCount = 60;
+ var bigListing = new FileSystemMetadata[5000];
+ for (var i = 0; i < bigListing.Length; i++)
+ {
+ bigListing[i] = new FileSystemMetadata
+ {
+ FullName = "/music/track" + i.ToString(CultureInfo.InvariantCulture),
+ IsDirectory = false
+ };
+ }
+
+ var fileSystemMock = new Mock<IFileSystem>();
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(It.IsAny<string>()))
+ .Returns(bigListing);
+
+ var directoryService = new DirectoryService(fileSystemMock.Object);
+
+ const string FirstPath = "/music/artist0";
+ directoryService.GetFileSystemEntries(FirstPath);
+
+ for (var i = 1; i < FolderCount; i++)
+ {
+ directoryService.GetFileSystemEntries("/music/artist" + i.ToString(CultureInfo.InvariantCulture));
+ }
+
+ directoryService.GetFileSystemEntries(FirstPath);
+
+ fileSystemMock.Verify(f => f.GetFileSystemEntries(FirstPath), Times.Exactly(2));
+ }
+
+ [Fact]
+ public void GetFileSystemEntries_RepeatedlyInvalidatedFolder_KeepsUnrelatedEntriesCached()
+ {
+ // Invalidating gives the records back, so churning one folder must not add up to the
+ // ceiling and drop everything else with it.
+ const int ChurnCount = 50;
+ var bigListing = new FileSystemMetadata[5000];
+ for (var i = 0; i < bigListing.Length; i++)
+ {
+ bigListing[i] = new FileSystemMetadata
+ {
+ FullName = "/music/track" + i.ToString(CultureInfo.InvariantCulture),
+ IsDirectory = false
+ };
+ }
+
+ var fileSystemMock = new Mock<IFileSystem>();
+ fileSystemMock.Setup(f => f.GetFileSystemEntries(It.IsAny<string>()))
+ .Returns(bigListing);
+
+ var directoryService = new DirectoryService(fileSystemMock.Object);
+
+ const string ChurnedPath = "/music/watched";
+ const string StablePath = "/music/untouched";
+ directoryService.GetFileSystemEntries(StablePath);
+
+ for (var i = 0; i < ChurnCount; i++)
+ {
+ directoryService.GetFileSystemEntries(ChurnedPath);
+ directoryService.Invalidate(ChurnedPath);
+ }
+
+ directoryService.GetFileSystemEntries(StablePath);
+
+ fileSystemMock.Verify(f => f.GetFileSystemEntries(StablePath), Times.Once);
+ }
+
+ [Fact]
+ public void GetFileSystemEntry_MissingPath_IsNotRemembered()
+ {
+ const string MissingPath = "/music/not-here";
+
+ var fileSystemMock = new Mock<IFileSystem>();
+ fileSystemMock.SetupSequence(f => f.GetFileSystemInfo(MissingPath))
+ .Returns(new FileSystemMetadata { FullName = MissingPath, Exists = false })
+ .Returns(new FileSystemMetadata { FullName = MissingPath, Exists = true });
+
+ var directoryService = new DirectoryService(fileSystemMock.Object);
+
+ Assert.Null(directoryService.GetFileSystemEntry(MissingPath));
+
+ Assert.NotNull(directoryService.GetFileSystemEntry(MissingPath));
+ }
+
+ private static string LocalPath(string path)
+ => path.Replace('/', Path.DirectorySeparatorChar);
}
}
diff --git a/tests/Jellyfin.Controller.Tests/LibraryTaskScheduler/LimitedConcurrencyLibrarySchedulerTests.cs b/tests/Jellyfin.Controller.Tests/LibraryTaskScheduler/LimitedConcurrencyLibrarySchedulerTests.cs
new file mode 100644
index 0000000000..686d839f4f
--- /dev/null
+++ b/tests/Jellyfin.Controller.Tests/LibraryTaskScheduler/LimitedConcurrencyLibrarySchedulerTests.cs
@@ -0,0 +1,216 @@
+using System;
+using System.Collections.Concurrent;
+using System.Diagnostics;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.LibraryTaskScheduler;
+using MediaBrowser.Model.Configuration;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Controller.Tests.LibraryTaskScheduler
+{
+ public class LimitedConcurrencyLibrarySchedulerTests
+ {
+ private static readonly TimeSpan _shortGracePeriod = TimeSpan.FromMilliseconds(50);
+
+ // Generous, because these only ever wait for something that should already have happened.
+ private static readonly TimeSpan _timeout = TimeSpan.FromSeconds(10);
+
+ [Fact]
+ public async Task Enqueue_ProcessesEveryItem()
+ {
+ using var appStopping = new CancellationTokenSource();
+ var scheduler = CreateScheduler(appStopping);
+ await using (scheduler)
+ {
+ var data = Enumerable.Range(0, 100).ToArray();
+ var processed = new ConcurrentBag<int>();
+
+ await scheduler.Enqueue(
+ data,
+ (item, _) =>
+ {
+ processed.Add(item);
+ return Task.CompletedTask;
+ },
+ new Progress<double>(),
+ CancellationToken.None);
+
+ Assert.Equal(data, processed.Order());
+ }
+ }
+
+ [Fact]
+ public async Task Enqueue_WithFailingWorker_StillCompletes()
+ {
+ using var appStopping = new CancellationTokenSource();
+ var scheduler = CreateScheduler(appStopping);
+ await using (scheduler)
+ {
+ await scheduler.Enqueue(
+ Enumerable.Range(0, 20).ToArray(),
+ (item, _) => item % 2 == 0 ? throw new InvalidOperationException("boom") : Task.CompletedTask,
+ new Progress<double>(),
+ CancellationToken.None);
+ }
+ }
+
+ /// <summary>
+ /// The runners wait on a source linked to <see cref="IHostApplicationLifetime.ApplicationStopping"/>,
+ /// so a shutdown has to reach them. It does not travel from the linked source back to the one
+ /// the cleanup cancels, which is what made them immortal.
+ /// </summary>
+ /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+ [Fact]
+ public async Task ApplicationStopping_RetiresRunners()
+ {
+ using var appStopping = new CancellationTokenSource();
+
+ // Long enough that the cleanup cannot be what retires them.
+ var scheduler = CreateScheduler(appStopping, gracePeriod: TimeSpan.FromMinutes(5));
+ await using (scheduler)
+ {
+ await RunOneOperation(scheduler);
+ Assert.True(scheduler.ActiveRunnerCount > 0);
+
+ await appStopping.CancelAsync();
+
+ await WaitForAsync(() => scheduler.ActiveRunnerCount == 0);
+ }
+ }
+
+ /// <summary>
+ /// The cleanup used to be a one shot: it never released the scheduling slot it took, so
+ /// every runner spawned after the first pass stayed around for the lifetime of the server.
+ /// </summary>
+ /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+ [Fact]
+ public async Task Enqueue_RetiresIdleRunnersAfterEveryOperation()
+ {
+ using var appStopping = new CancellationTokenSource();
+ var scheduler = CreateScheduler(appStopping);
+ await using (scheduler)
+ {
+ for (var round = 0; round < 3; round++)
+ {
+ await RunOneOperation(scheduler);
+ Assert.True(scheduler.ActiveRunnerCount > 0, $"no runner spawned in round {round}");
+
+ await WaitForAsync(() => scheduler.ActiveRunnerCount == 0);
+ }
+ }
+ }
+
+ /// <summary>
+ /// Disposing used to sit out the rest of the cleanup grace period, holding up shutdown for
+ /// up to a minute.
+ /// </summary>
+ /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+ [Fact]
+ public async Task DisposeAsync_DoesNotWaitOutTheGracePeriod()
+ {
+ using var appStopping = new CancellationTokenSource();
+ var scheduler = CreateScheduler(appStopping, gracePeriod: TimeSpan.FromMinutes(5));
+
+ await RunOneOperation(scheduler);
+
+ var stopwatch = Stopwatch.StartNew();
+ await scheduler.DisposeAsync();
+
+ Assert.True(stopwatch.Elapsed < _timeout, $"disposing took {stopwatch.Elapsed}");
+ }
+
+ [Fact]
+ public async Task Enqueue_AfterDispose_DoesNothing()
+ {
+ using var appStopping = new CancellationTokenSource();
+ var scheduler = CreateScheduler(appStopping);
+ await scheduler.DisposeAsync();
+
+ var processed = 0;
+ await scheduler.Enqueue(
+ Enumerable.Range(0, 10).ToArray(),
+ (_, _) =>
+ {
+ Interlocked.Increment(ref processed);
+ return Task.CompletedTask;
+ },
+ new Progress<double>(),
+ CancellationToken.None);
+
+ Assert.Equal(0, processed);
+ }
+
+ [Theory]
+ [InlineData(1)]
+ [InlineData(4)]
+ public async Task Enqueue_FromWithinAWorker_DoesNotDeadlock(int fanout)
+ {
+ using var appStopping = new CancellationTokenSource();
+ var scheduler = CreateScheduler(appStopping, fanout: fanout);
+ await using (scheduler)
+ {
+ var inner = 0;
+
+ var outer = scheduler.Enqueue(
+ Enumerable.Range(0, 8).ToArray(),
+ (_, _) => scheduler.Enqueue(
+ Enumerable.Range(0, 4).ToArray(),
+ (_, _) =>
+ {
+ Interlocked.Increment(ref inner);
+ return Task.CompletedTask;
+ },
+ new Progress<double>(),
+ CancellationToken.None),
+ new Progress<double>(),
+ CancellationToken.None);
+
+ await outer.WaitAsync(_timeout, TestContext.Current.CancellationToken);
+
+ Assert.Equal(32, inner);
+ }
+ }
+
+ private static LimitedConcurrencyLibraryScheduler CreateScheduler(
+ CancellationTokenSource appStopping,
+ int fanout = 4,
+ TimeSpan? gracePeriod = null)
+ {
+ var lifetime = new Mock<IHostApplicationLifetime>();
+ lifetime.SetupGet(x => x.ApplicationStopping).Returns(() => appStopping.Token);
+
+ var configurationManager = new Mock<IServerConfigurationManager>();
+ configurationManager.SetupGet(x => x.Configuration)
+ .Returns(new ServerConfiguration { LibraryScanFanoutConcurrency = fanout });
+
+ return new LimitedConcurrencyLibraryScheduler(
+ lifetime.Object,
+ NullLogger<LimitedConcurrencyLibraryScheduler>.Instance,
+ configurationManager.Object,
+ gracePeriod ?? _shortGracePeriod);
+ }
+
+ private static Task RunOneOperation(LimitedConcurrencyLibraryScheduler scheduler)
+ => scheduler.Enqueue(
+ Enumerable.Range(0, 8).ToArray(),
+ (_, _) => Task.CompletedTask,
+ new Progress<double>(),
+ CancellationToken.None);
+
+ private static async Task WaitForAsync(Func<bool> condition)
+ {
+ var stopwatch = Stopwatch.StartNew();
+ while (!condition())
+ {
+ Assert.True(stopwatch.Elapsed < _timeout, "timed out waiting for the scheduler to settle");
+ await Task.Delay(20, TestContext.Current.CancellationToken);
+ }
+ }
+ }
+}
diff --git a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs
index 5749944fcd..248b236df8 100644
--- a/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs
+++ b/tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs
@@ -1,4 +1,5 @@
using System;
+using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
@@ -377,6 +378,116 @@ namespace Jellyfin.Providers.Tests.Manager
GetMetadataProviders_CanRefreshMetadata_Tester(providerType, expected, ownedItem: true);
}
+ [Fact]
+ public async Task QueueRefresh_ManyItemsQueuedFromManyThreads_ProcessesEveryOne()
+ {
+ const int ItemCount = 2000;
+
+ var queued = Enumerable.Range(0, ItemCount).Select(_ => Guid.NewGuid()).ToArray();
+ var processed = new ConcurrentBag<Guid>();
+ var allProcessed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+
+ var libraryManager = new Mock<ILibraryManager>();
+ libraryManager.Setup(i => i.GetItemById(It.IsAny<Guid>()))
+ .Returns((Guid id) =>
+ {
+ // Returning null drains the entry without the whole refresh machinery.
+ processed.Add(id);
+ if (processed.Count == ItemCount)
+ {
+ allProcessed.TrySetResult();
+ }
+
+ return null;
+ });
+
+ using var providerManager = GetProviderManager(libraryManager: libraryManager.Object);
+
+ await Parallel.ForEachAsync(
+ queued,
+ TestContext.Current.CancellationToken,
+ (id, _) =>
+ {
+ providerManager.QueueRefresh(id, new MetadataRefreshOptions(Mock.Of<IDirectoryService>()), RefreshPriority.Normal);
+ return ValueTask.CompletedTask;
+ });
+
+ using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken);
+ timeout.CancelAfter(TimeSpan.FromSeconds(30));
+
+ try
+ {
+ await allProcessed.Task.WaitAsync(timeout.Token);
+ }
+ catch (OperationCanceledException)
+ {
+ // Fall through so the assertions report what was lost.
+ }
+
+ Assert.Empty(providerManager.GetRefreshQueue());
+ Assert.Equal(queued.Order().ToArray(), processed.Order().ToArray());
+ }
+
+ [Fact]
+ public async Task QueueRefresh_RefreshCancelsForItsOwnReasons_KeepsDrainingTheQueue()
+ {
+ // A provider timeout arrives as an OperationCanceledException, indistinguishable from
+ // a shutdown; treating it as one would strand the rest of the queue.
+ const int ItemCount = 200;
+
+ var queued = Enumerable.Range(0, ItemCount).Select(_ => Guid.NewGuid()).ToArray();
+ var processed = new ConcurrentBag<Guid>();
+ var allProcessed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+ using var allQueued = new ManualResetEventSlim(false);
+ var cancelledOnce = false;
+
+ var libraryManager = new Mock<ILibraryManager>();
+ libraryManager.Setup(i => i.GetItemById(It.IsAny<Guid>()))
+ .Returns((Guid id) =>
+ {
+ if (!cancelledOnce)
+ {
+ cancelledOnce = true;
+
+ // Hold the first entry until the whole batch is queued.
+ allQueued.Wait(TimeSpan.FromSeconds(30));
+ throw new OperationCanceledException("provider timed out");
+ }
+
+ processed.Add(id);
+ if (processed.Count == ItemCount - 1)
+ {
+ allProcessed.TrySetResult();
+ }
+
+ return null;
+ });
+
+ using var providerManager = GetProviderManager(libraryManager: libraryManager.Object);
+
+ foreach (var id in queued)
+ {
+ providerManager.QueueRefresh(id, new MetadataRefreshOptions(Mock.Of<IDirectoryService>()), RefreshPriority.Normal);
+ }
+
+ allQueued.Set();
+
+ using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken);
+ timeout.CancelAfter(TimeSpan.FromSeconds(30));
+
+ try
+ {
+ await allProcessed.Task.WaitAsync(timeout.Token);
+ }
+ catch (OperationCanceledException)
+ {
+ // Fall through so the assertions report what was stranded.
+ }
+
+ Assert.Empty(providerManager.GetRefreshQueue());
+ Assert.Equal(ItemCount - 1, processed.Count);
+ }
+
private static void GetMetadataProviders_CanRefreshMetadata_Tester(
string providerType,
bool expected,
@@ -554,15 +665,20 @@ namespace Jellyfin.Providers.Tests.Manager
private static ProviderManager GetProviderManager(
ServerConfiguration? serverConfiguration = null,
LibraryOptions? libraryOptions = null,
- IBaseItemManager? baseItemManager = null)
+ IBaseItemManager? baseItemManager = null,
+ ILibraryManager? libraryManager = null)
{
var serverConfigurationManager = new Mock<IServerConfigurationManager>(MockBehavior.Strict);
serverConfigurationManager.Setup(i => i.Configuration)
.Returns(serverConfiguration ?? new ServerConfiguration());
- var libraryManager = new Mock<ILibraryManager>(MockBehavior.Strict);
- libraryManager.Setup(i => i.GetLibraryOptions(It.IsAny<BaseItem>()))
- .Returns(libraryOptions ?? new LibraryOptions());
+ if (libraryManager is null)
+ {
+ var libraryManagerMock = new Mock<ILibraryManager>(MockBehavior.Strict);
+ libraryManagerMock.Setup(i => i.GetLibraryOptions(It.IsAny<BaseItem>()))
+ .Returns(libraryOptions ?? new LibraryOptions());
+ libraryManager = libraryManagerMock.Object;
+ }
var providerManager = new ProviderManager(
Mock.Of<IHttpClientFactory>(),
@@ -572,7 +688,7 @@ namespace Jellyfin.Providers.Tests.Manager
_logger,
Mock.Of<IFileSystem>(),
Mock.Of<IServerApplicationPaths>(),
- libraryManager.Object,
+ libraryManager,
baseItemManager!,
Mock.Of<ILyricManager>(),
Mock.Of<IMemoryCache>(),
diff --git a/tests/Jellyfin.Server.Integration.Tests/Controllers/LibraryStructureControllerTests.cs b/tests/Jellyfin.Server.Integration.Tests/Controllers/LibraryStructureControllerTests.cs
index 0a5838c545..79b9d1e2c5 100644
--- a/tests/Jellyfin.Server.Integration.Tests/Controllers/LibraryStructureControllerTests.cs
+++ b/tests/Jellyfin.Server.Integration.Tests/Controllers/LibraryStructureControllerTests.cs
@@ -6,8 +6,11 @@ using System.Text.Json;
using System.Threading.Tasks;
using Jellyfin.Api.Models.LibraryStructureDto;
using Jellyfin.Extensions.Json;
+using MediaBrowser.Controller;
+using MediaBrowser.Controller.Providers;
using MediaBrowser.Model.Configuration;
using MediaBrowser.Model.Entities;
+using Microsoft.Extensions.DependencyInjection;
using Xunit;
using Xunit.v3.Priority;
@@ -26,6 +29,45 @@ public sealed class LibraryStructureControllerTests : IClassFixture<JellyfinAppl
}
[Fact]
+ [Priority(-3)]
+ public async Task AddVirtualFolder_WithWarmDirectoryServiceCache_InvalidatesTheParentListing()
+ {
+ const string Name = "stale-cache-test";
+
+ var client = _factory.CreateClient();
+ client.DefaultRequestHeaders.AddAuthHeader(_accessToken ??= await AuthHelper.CompleteStartupAsync(client));
+
+ var directoryService = _factory.Services.GetRequiredService<IDirectoryService>();
+ var rootFolderPath = _factory.Services.GetRequiredService<IServerApplicationPaths>().DefaultUserViewsPath;
+
+ // Cache a listing of the libraries root taken before the new folder exists. Everything
+ // resolving through this DirectoryService keeps reading that listing until it is dropped,
+ // so the library stays invisible. Making the caches shared once turned this into a real
+ // test failure, see UpdateLibraryOptions_Valid_Success.
+ Assert.DoesNotContain(
+ directoryService.GetFileSystemEntries(rootFolderPath),
+ x => string.Equals(x.Name, Name, StringComparison.Ordinal));
+
+ var body = new AddVirtualFolderDto()
+ {
+ LibraryOptions = new LibraryOptions()
+ {
+ Enabled = false
+ }
+ };
+
+ using var response = await client.PostAsJsonAsync($"Library/VirtualFolders?name={Name}&refreshLibrary=false", body, _jsonOptions, TestContext.Current.CancellationToken);
+ Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
+
+ Assert.Contains(
+ directoryService.GetFileSystemEntries(rootFolderPath),
+ x => string.Equals(x.Name, Name, StringComparison.Ordinal));
+
+ using var cleanup = await client.DeleteAsync($"Library/VirtualFolders?name={Name}&refreshLibrary=false", TestContext.Current.CancellationToken);
+ Assert.Equal(HttpStatusCode.NoContent, cleanup.StatusCode);
+ }
+
+ [Fact]
[Priority(-1)]
public async Task Post_NewVirtualFolder_NotFound()
{