Merge pull request #17800 from fmarcac/fix/syncplay-active-session-counter

Fix SyncPlay active session counter leaking on rejoin

2 files changed, 98 insertions, 3 deletions

diff --git a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs
index b45d754554..a2b9088708 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;
                         }
@@ -400,7 +400,7 @@ namespace Emby.Server.Implementations.SyncPlay
             // Update sessions counter.
             var newSessionsCounter = _activeUsers.AddOrUpdate(
                 userId,
-                1,
+                toAdd,
                 (_, sessionsCounter) => sessionsCounter + toAdd);
 
             // Should never happen.
diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/SyncPlayManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/SyncPlayManagerTests.cs
new file mode 100644
index 0000000000..b1221f6f71
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/SyncPlayManagerTests.cs
@@ -0,0 +1,95 @@
+using System;
+using System.Threading;
+using Jellyfin.Database.Implementations.Entities;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Session;
+using MediaBrowser.Controller.SyncPlay.Requests;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+using SyncPlayManager = Emby.Server.Implementations.SyncPlay.SyncPlayManager;
+
+namespace Jellyfin.Server.Implementations.Tests.SyncPlay;
+
+public class SyncPlayManagerTests
+{
+    [Fact]
+    public void LeaveGroup_AfterJoiningTheSameGroupTwice_ClearsTheActiveSessionCounter()
+    {
+        var harness = new ManagerHarness();
+
+        var info = harness.Manager.NewGroup(harness.Session, new NewGroupRequest("group"), CancellationToken.None);
+        Assert.True(harness.Manager.IsUserActive(harness.User.Id));
+
+        // A client that re-sends Join for the group it is already in must not be counted twice.
+        harness.Manager.JoinGroup(harness.Session, new JoinGroupRequest(info.GroupId), CancellationToken.None);
+        harness.Manager.LeaveGroup(harness.Session, new LeaveGroupRequest(), CancellationToken.None);
+
+        Assert.False(harness.Manager.IsUserActive(harness.User.Id));
+    }
+
+    [Fact]
+    public void LeaveGroup_AfterASingleJoin_ClearsTheActiveSessionCounter()
+    {
+        var harness = new ManagerHarness();
+
+        harness.Manager.NewGroup(harness.Session, new NewGroupRequest("group"), CancellationToken.None);
+        harness.Manager.LeaveGroup(harness.Session, new LeaveGroupRequest(), CancellationToken.None);
+
+        Assert.False(harness.Manager.IsUserActive(harness.User.Id));
+    }
+
+    [Fact]
+    public void IsUserActive_WithTwoSessionsOfTheSameUser_TracksBothSeparately()
+    {
+        var harness = new ManagerHarness();
+        var second = harness.CreateSession("session-2");
+
+        var info = harness.Manager.NewGroup(harness.Session, new NewGroupRequest("group"), CancellationToken.None);
+        harness.Manager.JoinGroup(second, new JoinGroupRequest(info.GroupId), CancellationToken.None);
+
+        harness.Manager.LeaveGroup(harness.Session, new LeaveGroupRequest(), CancellationToken.None);
+        Assert.True(harness.Manager.IsUserActive(harness.User.Id));
+
+        harness.Manager.LeaveGroup(second, new LeaveGroupRequest(), CancellationToken.None);
+        Assert.False(harness.Manager.IsUserActive(harness.User.Id));
+    }
+
+    private sealed class ManagerHarness
+    {
+        private readonly Mock<ISessionManager> _sessionManager = new();
+
+        public ManagerHarness()
+        {
+            var userManager = new Mock<IUserManager>();
+            var libraryManager = new Mock<ILibraryManager>();
+
+            User = new User("tester", "auth-provider", "pwdreset-provider");
+            userManager.Setup(m => m.GetUserById(It.IsAny<Guid>())).Returns(User);
+
+            Manager = new SyncPlayManager(
+                NullLoggerFactory.Instance,
+                userManager.Object,
+                _sessionManager.Object,
+                libraryManager.Object);
+
+            Session = CreateSession("session-1");
+        }
+
+        public SyncPlayManager Manager { get; }
+
+        public User User { get; }
+
+        public SessionInfo Session { get; }
+
+        public SessionInfo CreateSession(string id)
+        {
+            return new SessionInfo(_sessionManager.Object, NullLogger.Instance)
+            {
+                Id = id,
+                UserId = User.Id,
+                UserName = User.Username
+            };
+        }
+    }
+}