Backport pull request #18020 from jellyfin/release-12.z

Stop the library monitor from refreshing against a disposed host

Original-merge: 3aec32cb24

Merged-by: crobibero <cody@robibe.ro>

Backported-by: Cody Robibero <cody@robibe.ro>

Emby.Server.Implementations/IO/FileRefresher.cs

@@ -109,6 +109,11 @@ namespace Emby.Server.Implementations.IO
 
             lock (_timerLock)
             {
+                if (_disposed)
+                {
+                    return;
+                }
+
                 paths = _affectedPaths.ToList();
             }
 
@@ -129,11 +134,12 @@ namespace Emby.Server.Implementations.IO
 
         private void ProcessPathChanges(List<string> paths)
         {
-            IEnumerable<BaseItem> itemsToRefresh = paths
+            var itemsToRefresh = paths
                 .Distinct()
-                .Select(GetAffectedBaseItem)
-                .Where(item => item is not null)
-                .DistinctBy(x => x!.Id)!;  // Removed null values in the previous .Where()
+                .Select(TryGetAffectedBaseItem)
+                .OfType<BaseItem>()
+                .DistinctBy(x => x.Id)
+                .ToList();
 
             foreach (var item in itemsToRefresh)
             {
@@ -155,6 +161,19 @@ namespace Emby.Server.Implementations.IO
             }
         }
 
+        private BaseItem? TryGetAffectedBaseItem(string path)
+        {
+            try
+            {
+                return GetAffectedBaseItem(path);
+            }
+            catch (Exception ex)
+            {
+                _logger.LogError(ex, "Error finding the item affected by changes to {Path}", path);
+                return null;
+            }
+        }
+
         /// <summary>
         /// Gets the affected base item.
         /// </summary>

Emby.Server.Implementations/IO/LibraryMonitor.cs

@@ -3,6 +3,7 @@ using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using Emby.Server.Implementations.Library;
 using MediaBrowser.Controller.Configuration;
@@ -40,6 +41,12 @@ namespace Emby.Server.Implementations.IO
         /// </summary>
         private readonly ConcurrentDictionary<string, string> _tempIgnoredPaths = new(StringComparer.OrdinalIgnoreCase);
 
+        /// <summary>
+        /// Incremented by every <see cref="Stop"/> so watchers still being created on a background
+        /// task can tell that the sweep they should have been caught by has already run.
+        /// </summary>
+        private int _watcherGeneration;
+
         private bool _disposed;
 
         /// <summary>
@@ -69,7 +76,7 @@ namespace Emby.Server.Implementations.IO
             _dotIgnoreIgnoreRule = dotIgnoreIgnoreRule;
 
             appLifetime.ApplicationStarted.Register(Start);
-            appLifetime.ApplicationStopping.Register(Stop);
+            appLifetime.ApplicationStopping.Register(Dispose);
         }
 
         /// <inheritdoc />
@@ -120,6 +127,11 @@ namespace Emby.Server.Implementations.IO
         /// <inheritdoc />
         public void Start()
         {
+            if (_disposed)
+            {
+                return;
+            }
+
             _libraryManager.ItemAdded += OnLibraryManagerItemAdded;
             _libraryManager.ItemRemoved += OnLibraryManagerItemRemoved;
 
@@ -233,6 +245,8 @@ namespace Emby.Server.Implementations.IO
                 return;
             }
 
+            var generation = Volatile.Read(ref _watcherGeneration);
+
             // Creating a FileSystemWatcher over the LAN can take hundreds of milliseconds, so wrap it in a Task to do them all in parallel
             Task.Run(() =>
             {
@@ -256,7 +270,11 @@ namespace Emby.Server.Implementations.IO
                     newWatcher.Changed += OnWatcherChanged;
                     newWatcher.Error += OnWatcherError;
 
-                    if (_fileSystemWatchers.TryAdd(path, newWatcher))
+                    if (_disposed || Volatile.Read(ref _watcherGeneration) != generation)
+                    {
+                        DisposeWatcher(newWatcher, false);
+                    }
+                    else if (_fileSystemWatchers.TryAdd(path, newWatcher))
                     {
                         newWatcher.EnableRaisingEvents = true;
                         _logger.LogInformation("Watching directory {Path}", path);
@@ -357,6 +375,11 @@ namespace Emby.Server.Implementations.IO
         {
             ArgumentException.ThrowIfNullOrEmpty(path);
 
+            if (_disposed)
+            {
+                return;
+            }
+
             if (IgnorePatterns.ShouldIgnore(path))
             {
                 return;
@@ -452,6 +475,8 @@ namespace Emby.Server.Implementations.IO
         /// </summary>
         public void Stop()
         {
+            Interlocked.Increment(ref _watcherGeneration);
+
             _libraryManager.ItemAdded -= OnLibraryManagerItemAdded;
             _libraryManager.ItemRemoved -= OnLibraryManagerItemRemoved;
 
@@ -496,8 +521,9 @@ namespace Emby.Server.Implementations.IO
                 return;
             }
 
-            Stop();
+            // Set before stopping so anything racing us stops handing out new work.
             _disposed = true;
+            Stop();
         }
     }
 }

tests/Jellyfin.Server.Implementations.Tests/IO/FileRefresherTests.cs

@@ -0,0 +1,57 @@
+using System;
+using System.IO;
+using System.Threading.Tasks;
+using Emby.Server.Implementations.IO;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Model.Configuration;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.IO;
+
+public class FileRefresherTests
+{
+    [Fact]
+    public async Task ProcessPathChanges_PathLookupThrows_StillRefreshesRemainingPaths()
+    {
+        var tempDir = Directory.CreateTempSubdirectory("filerefresher");
+        try
+        {
+            // Ordered so the failing path is dequeued first.
+            var failingPath = Path.Combine(tempDir.FullName, "failing", "episode.mkv");
+            var workingPath = Directory.CreateDirectory(Path.Combine(tempDir.FullName, "working")).FullName;
+
+            var workingItem = new Folder { Path = workingPath, Name = "working" };
+            var workingItemFound = new TaskCompletionSource();
+
+            var libraryManager = new Mock<ILibraryManager>(MockBehavior.Loose);
+            libraryManager.Setup(x => x.FindByPath(failingPath, null))
+                .Throws(new ObjectDisposedException("IServiceProvider"));
+            libraryManager.Setup(x => x.FindByPath(workingPath, null))
+                .Returns(workingItem)
+                .Callback(() => workingItemFound.TrySetResult());
+
+            var configurationManager = new Mock<IServerConfigurationManager>(MockBehavior.Loose);
+            configurationManager.Setup(x => x.Configuration)
+                .Returns(new ServerConfiguration { LibraryMonitorDelay = 1 });
+
+            using var refresher = new FileRefresher(
+                failingPath,
+                configurationManager.Object,
+                libraryManager.Object,
+                NullLogger.Instance);
+            refresher.AddPath(workingPath);
+
+            await workingItemFound.Task.WaitAsync(TimeSpan.FromSeconds(30), TestContext.Current.CancellationToken);
+
+            libraryManager.Verify(x => x.FindByPath(failingPath, null), Times.Once);
+        }
+        finally
+        {
+            tempDir.Delete(true);
+        }
+    }
+}