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

Fix SyncPlay active session counter leaking on rejoin
2026-09-05 10:57:55 -04:00
parent d4a23cd438 51a7d5d08a
commit 2b0ccf2a30
2 changed files with 98 additions and 3 deletions
@@ -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.
@@ -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
+            };
+        }
+    }
+}