Merge branch 'master' into fix-code-migration

5 files changed, 109 insertions, 59 deletions

diff --git a/Emby.Server.Implementations/EntryPoints/LibraryChangedNotifier.cs b/Emby.Server.Implementations/EntryPoints/LibraryChangedNotifier.cs
index 933cfc8cbe..02b104756e 100644
--- a/Emby.Server.Implementations/EntryPoints/LibraryChangedNotifier.cs
+++ b/Emby.Server.Implementations/EntryPoints/LibraryChangedNotifier.cs
@@ -27,6 +27,11 @@ namespace Emby.Server.Implementations.EntryPoints;
 /// </summary>
 public sealed class LibraryChangedNotifier : IHostedService, IDisposable
 {
+    // A batch holds a live reference to every item it names, so it has to stay small enough that a
+    // library scan - which changes items faster than any batch window closes - cannot grow it without
+    // bound. Reached only by a scan; interactive use closes a batch on the window long before this.
+    internal const int MaxBatchSize = 2000;
+
     private readonly ILibraryManager _libraryManager;
     private readonly IServerConfigurationManager _configurationManager;
     private readonly IProviderManager _providerManager;
@@ -35,11 +40,11 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable
     private readonly ILogger<LibraryChangedNotifier> _logger;
 
     private readonly Lock _libraryChangedSyncLock = new();
-    private readonly List<Folder> _foldersAddedTo = new();
-    private readonly List<Folder> _foldersRemovedFrom = new();
-    private readonly List<BaseItem> _itemsAdded = new();
-    private readonly List<BaseItem> _itemsRemoved = new();
-    private readonly List<BaseItem> _itemsUpdated = new();
+    private readonly Dictionary<Guid, Folder> _foldersAddedTo = [];
+    private readonly Dictionary<Guid, Folder> _foldersRemovedFrom = [];
+    private readonly Dictionary<Guid, BaseItem> _itemsAdded = [];
+    private readonly Dictionary<Guid, BaseItem> _itemsRemoved = [];
+    private readonly Dictionary<Guid, BaseItem> _itemsUpdated = [];
     private readonly ConcurrentDictionary<Guid, DateTime> _lastProgressMessageTimes = new();
 
     private Timer? _libraryUpdateTimer;
@@ -173,7 +178,7 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable
     private void OnLibraryItemRemoved(object? sender, ItemChangeEventArgs e)
         => OnLibraryChange(e.Item, e.Parent, _itemsRemoved, _foldersRemovedFrom);
 
-    private void OnLibraryChange(BaseItem item, BaseItem parent, List<BaseItem> itemsList, List<Folder>? foldersList)
+    private void OnLibraryChange(BaseItem item, BaseItem parent, Dictionary<Guid, BaseItem> itemsList, Dictionary<Guid, Folder>? foldersList)
     {
         if (!FilterItem(item))
         {
@@ -182,23 +187,28 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable
 
         lock (_libraryChangedSyncLock)
         {
-            var updateDuration = TimeSpan.FromSeconds(_configurationManager.Configuration.LibraryUpdateDuration);
-
+            // The window runs from the first change of a batch and is never extended. Extending it on
+            // every change would keep a library scan's batch open for the whole scan, and the batch
+            // holds the items it names alive, so it would grow to the size of the library.
             if (_libraryUpdateTimer is null)
             {
+                var updateDuration = TimeSpan.FromSeconds(_configurationManager.Configuration.LibraryUpdateDuration);
                 _libraryUpdateTimer = new Timer(LibraryUpdateTimerCallback, null, updateDuration, Timeout.InfiniteTimeSpan);
             }
-            else
-            {
-                _libraryUpdateTimer.Change(updateDuration, Timeout.InfiniteTimeSpan);
-            }
 
             if (foldersList is not null && parent is Folder folder)
             {
-                foldersList.Add(folder);
+                foldersList[folder.Id] = folder;
             }
 
-            itemsList.Add(item);
+            itemsList[item.Id] = item;
+
+            // A window long enough to cover a burst still has to give way once the batch is large
+            // enough to be worth sending on its own.
+            if (_itemsAdded.Count + _itemsRemoved.Count + _itemsUpdated.Count >= MaxBatchSize)
+            {
+                _libraryUpdateTimer.Change(TimeSpan.Zero, Timeout.InfiniteTimeSpan);
+            }
         }
     }
 
@@ -211,22 +221,16 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable
         List<BaseItem> itemsRemoved;
         lock (_libraryChangedSyncLock)
         {
-            // Remove dupes in case some were saved multiple times
-            foldersAddedTo = _foldersAddedTo
-                .DistinctBy(x => x.Id)
-                .ToList();
-
-            foldersRemovedFrom = _foldersRemovedFrom
-                .DistinctBy(x => x.Id)
-                .ToList();
+            foldersAddedTo = _foldersAddedTo.Values.ToList();
+            foldersRemovedFrom = _foldersRemovedFrom.Values.ToList();
 
             itemsUpdated = _itemsUpdated
-                .Where(i => !_itemsAdded.Contains(i))
-                .DistinctBy(x => x.Id)
+                .Where(e => !_itemsAdded.ContainsKey(e.Key))
+                .Select(e => e.Value)
                 .ToList();
 
-            itemsAdded = _itemsAdded.ToList();
-            itemsRemoved = _itemsRemoved.ToList();
+            itemsAdded = _itemsAdded.Values.ToList();
+            itemsRemoved = _itemsRemoved.Values.ToList();
 
             if (_libraryUpdateTimer is not null)
             {
@@ -241,6 +245,15 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable
             _foldersRemovedFrom.Clear();
         }
 
+        if (itemsAdded.Count == 0
+            && itemsUpdated.Count == 0
+            && itemsRemoved.Count == 0
+            && foldersAddedTo.Count == 0
+            && foldersRemovedFrom.Count == 0)
+        {
+            return;
+        }
+
         await SendChangeNotifications(itemsAdded, itemsUpdated, itemsRemoved, foldersAddedTo, foldersRemovedFrom, CancellationToken.None).ConfigureAwait(false);
     }
 
diff --git a/Emby.Server.Implementations/EntryPoints/UserDataChangeNotifier.cs b/Emby.Server.Implementations/EntryPoints/UserDataChangeNotifier.cs
index fc174b7c14..b182e5837b 100644
--- a/Emby.Server.Implementations/EntryPoints/UserDataChangeNotifier.cs
+++ b/Emby.Server.Implementations/EntryPoints/UserDataChangeNotifier.cs
@@ -18,15 +18,17 @@ namespace Emby.Server.Implementations.EntryPoints
     public sealed class UserDataChangeNotifier : IHostedService, IDisposable
     {
         private const int UpdateDuration = 500;
+        internal const int MaxBatchSize = 2000;
 
         private readonly ISessionManager _sessionManager;
         private readonly IUserDataManager _userDataManager;
         private readonly IUserManager _userManager;
 
-        private readonly Dictionary<Guid, List<BaseItem>> _changedItems = new();
+        private readonly Dictionary<Guid, Dictionary<Guid, BaseItem>> _changedItems = [];
         private readonly Lock _syncLock = new();
 
         private Timer? _updateTimer;
+        private int _changedItemCount;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="UserDataChangeNotifier"/> class.
@@ -69,50 +71,64 @@ namespace Emby.Server.Implementations.EntryPoints
 
             lock (_syncLock)
             {
-                if (_updateTimer is null)
+                // The window runs from the first change of a batch and is never extended, so a stream
+                // of changes that never pauses - a library scan - still closes its batches instead of
+                // holding every item it touched alive until the stream stops.
+                _updateTimer ??= new Timer(
+                    UpdateTimerCallback,
+                    null,
+                    UpdateDuration,
+                    Timeout.Infinite);
+
+                if (!_changedItems.TryGetValue(e.UserId, out Dictionary<Guid, BaseItem>? keys))
                 {
-                    _updateTimer = new Timer(
-                        UpdateTimerCallback,
-                        null,
-                        UpdateDuration,
-                        Timeout.Infinite);
-                }
-                else
-                {
-                    _updateTimer.Change(UpdateDuration, Timeout.Infinite);
-                }
-
-                if (!_changedItems.TryGetValue(e.UserId, out List<BaseItem>? keys))
-                {
-                    keys = new List<BaseItem>();
+                    keys = [];
                     _changedItems[e.UserId] = keys;
                 }
 
-                keys.Add(e.Item);
-
                 var baseItem = e.Item;
 
                 // Go up one level for indicators
                 if (baseItem is not null)
                 {
+                    Track(keys, baseItem);
+
                     var parent = baseItem.GetOwner() ?? baseItem.GetParent();
 
                     if (parent is not null)
                     {
-                        keys.Add(parent);
+                        Track(keys, parent);
                     }
                 }
+
+                // A window long enough to cover a burst still has to give way once the batch is
+                // large enough to be worth sending on its own.
+                if (_changedItemCount >= MaxBatchSize)
+                {
+                    _updateTimer.Change(0, Timeout.Infinite);
+                }
+            }
+        }
+
+        private void Track(Dictionary<Guid, BaseItem> keys, BaseItem item)
+        {
+            var before = keys.Count;
+            keys[item.Id] = item;
+
+            if (keys.Count != before)
+            {
+                _changedItemCount++;
             }
         }
 
         private async void UpdateTimerCallback(object? state)
         {
-            List<KeyValuePair<Guid, List<BaseItem>>> changes;
+            List<KeyValuePair<Guid, Dictionary<Guid, BaseItem>>> changes;
             lock (_syncLock)
             {
-                // Remove dupes in case some were saved multiple times
                 changes = _changedItems.ToList();
                 _changedItems.Clear();
+                _changedItemCount = 0;
 
                 if (_updateTimer is not null)
                 {
@@ -121,17 +137,22 @@ namespace Emby.Server.Implementations.EntryPoints
                 }
             }
 
+            if (changes.Count == 0)
+            {
+                return;
+            }
+
             foreach (var (userId, changedItems) in changes)
             {
                 await _sessionManager.SendMessageToUserSessions(
                     [userId],
                     SessionMessageType.UserDataChanged,
-                    () => GetUserDataChangeInfo(userId, changedItems),
+                    () => GetUserDataChangeInfo(userId, changedItems.Values),
                     default).ConfigureAwait(false);
             }
         }
 
-        private UserDataChangeInfo GetUserDataChangeInfo(Guid userId, List<BaseItem> changedItems)
+        private UserDataChangeInfo GetUserDataChangeInfo(Guid userId, IEnumerable<BaseItem> changedItems)
         {
             var user = _userManager.GetUserById(userId)
                 ?? throw new ArgumentException("Invalid user ID", nameof(userId));
@@ -140,7 +161,6 @@ namespace Emby.Server.Implementations.EntryPoints
             {
                 UserId = userId,
                 UserDataList = changedItems
-                    .DistinctBy(x => x.Id)
                     .Select(i =>
                     {
                         var dto = _userDataManager.GetUserDataDto(i, user);
diff --git a/Emby.Server.Implementations/ScheduledTasks/Tasks/OptimizeDatabaseTask.cs b/Emby.Server.Implementations/ScheduledTasks/Tasks/OptimizeDatabaseTask.cs
index 8d133dc074..687947616f 100644
--- a/Emby.Server.Implementations/ScheduledTasks/Tasks/OptimizeDatabaseTask.cs
+++ b/Emby.Server.Implementations/ScheduledTasks/Tasks/OptimizeDatabaseTask.cs
@@ -11,7 +11,7 @@ using Microsoft.Extensions.Logging;
 namespace Emby.Server.Implementations.ScheduledTasks.Tasks;
 
 /// <summary>
-/// Optimizes Jellyfin's database by issuing a VACUUM command.
+/// Optimizes Jellyfin's database by issuing VACUUM and ANALYZE commands.
 /// </summary>
 public class OptimizeDatabaseTask : IScheduledTask, IConfigurableScheduledTask
 {
@@ -82,7 +82,7 @@ public class OptimizeDatabaseTask : IScheduledTask, IConfigurableScheduledTask
             return;
         }
 
-        _logger.LogInformation("Optimizing and vacuuming jellyfin.db...");
+        _logger.LogInformation("Vacuuming and analyzing jellyfin.db...");
 
         try
         {
diff --git a/Emby.Server.Implementations/SyncPlay/Group.cs b/Emby.Server.Implementations/SyncPlay/Group.cs
index 38a0018a70..923bfc67aa 100644
--- a/Emby.Server.Implementations/SyncPlay/Group.cs
+++ b/Emby.Server.Implementations/SyncPlay/Group.cs
@@ -91,6 +91,18 @@ namespace Emby.Server.Implementations.SyncPlay
         public long DefaultPing { get; } = 500;
 
         /// <summary>
+        /// Gets the maximum ping, in milliseconds, accepted from a session.
+        /// </summary>
+        /// <remarks>
+        /// Pings are reported by clients and are scaled into the delays used to schedule playback,
+        /// so an unbounded value lets a single session push the whole group's resume point
+        /// arbitrarily far out, or overflow the arithmetic entirely. Anything above this is not a
+        /// usable measurement for synchronisation.
+        /// </remarks>
+        /// <value>The maximum ping.</value>
+        public long MaxPing { get; } = 10000;
+
+        /// <summary>
         /// Gets the maximum time offset error accepted for dates reported by clients, in milliseconds.
         /// </summary>
         /// <value>The maximum time offset error.</value>
@@ -438,7 +450,7 @@ namespace Emby.Server.Implementations.SyncPlay
         {
             if (_participants.TryGetValue(session.Id, out GroupMember value))
             {
-                value.Ping = ping;
+                value.Ping = Math.Clamp(ping, 0, MaxPing);
             }
         }
 
@@ -451,7 +463,9 @@ namespace Emby.Server.Implementations.SyncPlay
                 max = Math.Max(max, session.Ping);
             }
 
-            return max;
+            // A group with no participants has no ping to report. Returning long.MinValue would
+            // overflow the callers that scale this value into ticks, so fall back to the default.
+            return max == long.MinValue ? DefaultPing : max;
         }
 
         /// <inheritdoc />
diff --git a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs
index b45d754554..b88ee33358 100644
--- a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs
+++ b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs
@@ -181,8 +181,8 @@ namespace Emby.Server.Implementations.SyncPlay
                     {
                         if (existingGroup.GroupId.Equals(request.GroupId))
                         {
-                            // Restore session.
-                            UpdateSessionsCounter(session.UserId, 1);
+                            // Restore session. The session is already in the group and has already
+                            // been counted, so the counter must not be incremented a second time.
                             group.SessionJoin(session, request, cancellationToken);
                             return;
                         }
@@ -332,8 +332,11 @@ namespace Emby.Server.Implementations.SyncPlay
                 // Group lock required as Group is not thread-safe.
                 lock (group)
                 {
-                    // Make sure that session still belongs to this group.
-                    if (_sessionToGroupMap.TryGetValue(session.Id, out var checkGroup) && !checkGroup.GroupId.Equals(group.GroupId))
+                    // Make sure that session still belongs to this group. The lookup can fail
+                    // outright when the session left while this request was waiting on the group
+                    // lock, which is exactly the case this re-check exists to catch.
+                    if (!_sessionToGroupMap.TryGetValue(session.Id, out var checkGroup)
+                        || !checkGroup.GroupId.Equals(group.GroupId))
                     {
                         // Drop request.
                         return;
@@ -400,7 +403,7 @@ namespace Emby.Server.Implementations.SyncPlay
             // Update sessions counter.
             var newSessionsCounter = _activeUsers.AddOrUpdate(
                 userId,
-                1,
+                toAdd,
                 (_, sessionsCounter) => sessionsCounter + toAdd);
 
             // Should never happen.