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

Fix SyncPlay authentication error handling and limit group member wait time

Original-merge: 5fcb65bb46

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

Backported-by: Cody Robibero <cody@robibe.ro>
This commit is contained in:
Shadowghost authored and Cody Robibero committed 2026-09-15 11:16:05 -04:00
1 parent fcd010d308
commit 93fc178db5
6 files changed
+458 -21

No files matched your search

+122 -3
View File
@@ -11,6 +11,7 @@ using MediaBrowser.Controller.Library;
using MediaBrowser.Controller.Session;
using MediaBrowser.Controller.SyncPlay;
using MediaBrowser.Controller.SyncPlay.GroupStates;
using MediaBrowser.Controller.SyncPlay.PlaybackRequests;
using MediaBrowser.Controller.SyncPlay.Queue;
using MediaBrowser.Controller.SyncPlay.Requests;
using MediaBrowser.Model.SyncPlay;
@@ -26,6 +27,11 @@ namespace Emby.Server.Implementations.SyncPlay
/// </remarks>
public class Group : IGroupStateContext
{
/// <summary>
/// The default value of <see cref="GroupWaitTimeout"/>, in milliseconds.
/// </summary>
internal const long DefaultGroupWaitTimeout = 30000;
/// <summary>
/// The logger.
/// </summary>
@@ -54,8 +60,12 @@ namespace Emby.Server.Implementations.SyncPlay
/// <summary>
/// The participants, or members of the group.
/// </summary>
private readonly Dictionary<string, GroupMember> _participants =
new Dictionary<string, GroupMember>(StringComparer.OrdinalIgnoreCase);
private readonly Dictionary<string, GroupMember> _participants = new(StringComparer.OrdinalIgnoreCase);
/// <summary>
/// The sessions of the participants, which only carry identifiers.
/// </summary>
private readonly Dictionary<string, SessionInfo> _participantSessions = new(StringComparer.OrdinalIgnoreCase);
/// <summary>
/// The internal group state.
@@ -114,6 +124,19 @@ namespace Emby.Server.Implementations.SyncPlay
/// <value>The maximum offset error.</value>
public long MaxPlaybackOffset { get; } = 500;
/// <summary>
/// Gets the maximum time, in milliseconds, the group waits for its members to report ready.
/// </summary>
/// <value>The group-wait timeout.</value>
internal long GroupWaitTimeout { get; init; } = DefaultGroupWaitTimeout;
/// <summary>
/// Gets the <see cref="Environment.TickCount64"/> value at which the group gives up waiting
/// for its members, or <c>null</c> when it is not waiting for anyone.
/// </summary>
/// <value>The group-wait deadline.</value>
internal long? GroupWaitDeadline { get; private set; }
/// <summary>
/// Gets the group identifier.
/// </summary>
@@ -163,6 +186,8 @@ namespace Emby.Server.Implementations.SyncPlay
Ping = DefaultPing,
IsBuffering = false
});
_participantSessions[session.Id] = session;
}
/// <summary>
@@ -172,6 +197,8 @@ namespace Emby.Server.Implementations.SyncPlay
private void RemoveSession(SessionInfo session)
{
_participants.Remove(session.Id);
_participantSessions.Remove(session.Id);
UpdateGroupWaitDeadline(false);
}
/// <summary>
@@ -389,13 +416,20 @@ namespace Emby.Server.Implementations.SyncPlay
{
value.IgnoreGroupWait = ignoreGroupWait;
}
UpdateGroupWaitDeadline(false);
}
/// <inheritdoc />
public void SetState(IGroupState state)
{
_logger.LogInformation("Group {GroupId} switching from {FromStateType} to {ToStateType}.", GroupId.ToString(), _state.Type, state.Type);
this._state = state;
_state = state;
if (state.Type != GroupStateType.Waiting)
{
GroupWaitDeadline = null;
}
}
/// <inheritdoc />
@@ -475,6 +509,8 @@ namespace Emby.Server.Implementations.SyncPlay
{
value.IsBuffering = isBuffering;
}
UpdateGroupWaitDeadline(false);
}
/// <inheritdoc />
@@ -484,6 +520,9 @@ namespace Emby.Server.Implementations.SyncPlay
{
session.IsBuffering = isBuffering;
}
// Resetting the status of every session starts a new waiting period.
UpdateGroupWaitDeadline(isBuffering);
}
/// <inheritdoc />
@@ -690,5 +729,85 @@ namespace Emby.Server.Implementations.SyncPlay
PlayQueue.ShuffleMode,
PlayQueue.RepeatMode);
}
/// <summary>
/// Stops waiting for the members that have not reported ready and lets the rest of the
/// group carry on. Does nothing until <see cref="GroupWaitDeadline"/> has passed.
/// </summary>
/// <param name="cancellationToken">The cancellation token.</param>
internal void HandleGroupWaitTimeout(CancellationToken cancellationToken)
{
var deadline = GroupWaitDeadline;
if (deadline is null || deadline > Environment.TickCount64)
{
return;
}
GroupWaitDeadline = null;
if (_state is not WaitingGroupState waitingState)
{
return;
}
var blockingSessions = _participantSessions
.Values
.Where(participant => _participants.TryGetValue(participant.Id, out var member)
&& member.IsBuffering
&& !member.IgnoreGroupWait)
.ToList();
if (blockingSessions.Count == 0)
{
return;
}
// The recovery below is broadcast to the whole group, so it does not matter which of
// the sessions that kept the group waiting is the one acting on the group's behalf.
var session = blockingSessions[0];
_logger.LogWarning(
"Group {GroupId} waited {Waited} ms for session(s) {SessionIds} to report ready, giving up.",
GroupId.ToString(),
GroupWaitTimeout + Environment.TickCount64 - deadline.Value,
string.Join(", ", blockingSessions.Select(participant => participant.Id)));
if (waitingState.ResumePlaying)
{
// An unpause request in the waiting state means "start now, ignoring the sessions
// that are not ready".
var unpauseRequest = new UnpauseGroupRequest();
waitingState.HandleRequest(unpauseRequest, this, GroupStateType.Waiting, session, cancellationToken);
return;
}
// The members have been paused for the whole waiting period, so the playback position
// stays where the wait started.
SetAllBuffering(false);
SetState(new PausedGroupState(_loggerFactory));
var command = NewSyncPlayCommand(SendCommandType.Pause);
SendCommand(session, SyncPlayBroadcastType.AllGroup, command, cancellationToken);
var stateUpdate = new GroupStateUpdate(GroupStateType.Paused, PlaybackRequestType.Pause);
var update = new SyncPlayStateUpdate(GroupId, stateUpdate);
SendGroupUpdate(session, SyncPlayBroadcastType.AllGroup, update, cancellationToken);
}
private void UpdateGroupWaitDeadline(bool startNewWaitingPeriod)
{
if (_state.Type != GroupStateType.Waiting || !IsBuffering())
{
GroupWaitDeadline = null;
return;
}
// A running deadline covers the waiting period as a whole, so the sessions that keep
// reporting buffering while they load must not push it back.
if (GroupWaitDeadline is null || startNewWaitingPeriod)
{
GroupWaitDeadline = Environment.TickCount64 + GroupWaitTimeout;
}
}
}
}
@@ -18,6 +18,11 @@ namespace Emby.Server.Implementations.SyncPlay
/// </summary>
public class SyncPlayManager : ISyncPlayManager, IDisposable
{
/// <summary>
/// How often, in milliseconds, the groups are checked for a spent wait deadline.
/// </summary>
private const int GroupWaitSweepInterval = 1000;
/// <summary>
/// The logger.
/// </summary>
@@ -69,6 +74,11 @@ namespace Emby.Server.Implementations.SyncPlay
/// </remarks>
private readonly Lock _groupsLock = new();
/// <summary>
/// The timer that watches the groups' wait deadlines, running only while there are groups.
/// </summary>
private readonly Timer _groupWaitTimer;
private bool _disposed = false;
/// <summary>
@@ -90,8 +100,15 @@ namespace Emby.Server.Implementations.SyncPlay
_libraryManager = libraryManager;
_logger = loggerFactory.CreateLogger<SyncPlayManager>();
_sessionManager.SessionEnded += OnSessionEnded;
_groupWaitTimer = new Timer(_ => OnGroupWaitTimerTick(), null, Timeout.Infinite, Timeout.Infinite);
}
/// <summary>
/// Gets the maximum time, in milliseconds, a group waits for its members to report ready.
/// </summary>
/// <value>The group-wait timeout.</value>
internal long GroupWaitTimeout { get; init; } = Group.DefaultGroupWaitTimeout;
/// <inheritdoc />
public void Dispose()
{
@@ -122,8 +139,12 @@ namespace Emby.Server.Implementations.SyncPlay
LeaveGroup(session, leaveGroupRequest, cancellationToken);
}
var group = new Group(_loggerFactory, _userManager, _sessionManager, _libraryManager);
var group = new Group(_loggerFactory, _userManager, _sessionManager, _libraryManager)
{
GroupWaitTimeout = GroupWaitTimeout
};
_groups[group.GroupId] = group;
UpdateGroupWaitTimer();
if (!_sessionToGroupMap.TryAdd(session.Id, group))
{
@@ -242,6 +263,7 @@ namespace Emby.Server.Implementations.SyncPlay
{
_logger.LogInformation("Group {GroupId} is empty, removing it.", group.GroupId);
_groups.Remove(group.GroupId, out _);
UpdateGroupWaitTimer();
}
}
}
@@ -384,7 +406,50 @@ namespace Emby.Server.Implementations.SyncPlay
}
_sessionManager.SessionEnded -= OnSessionEnded;
_disposed = true;
lock (_groupsLock)
{
_disposed = true;
_groupWaitTimer.Dispose();
}
}
private void UpdateGroupWaitTimer()
{
if (_disposed)
{
return;
}
var interval = _groups.IsEmpty ? Timeout.Infinite : GroupWaitSweepInterval;
_groupWaitTimer.Change(interval, interval);
}
private void OnGroupWaitTimerTick()
{
try
{
lock (_groupsLock)
{
if (_disposed)
{
return;
}
foreach (var (_, group) in _groups)
{
// Group lock required as Group is not thread-safe.
lock (group)
{
group.HandleGroupWaitTimeout(CancellationToken.None);
}
}
}
}
catch (Exception ex)
{
_logger.LogError(ex, "Error while recovering SyncPlay groups from a timed out wait.");
}
}
private void OnSessionEnded(object sender, SessionEventArgs e)
@@ -2,6 +2,7 @@ using System.Threading.Tasks;
using Jellyfin.Api.Extensions;
using Jellyfin.Data.Enums;
using Jellyfin.Database.Implementations.Enums;
using Jellyfin.Extensions;
using MediaBrowser.Common.Extensions;
using MediaBrowser.Controller.Library;
using MediaBrowser.Controller.SyncPlay;
@@ -34,6 +35,13 @@ namespace Jellyfin.Api.Auth.SyncPlayAccessPolicy
protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, SyncPlayAccessRequirement requirement)
{
var userId = context.User.GetUserId();
if (userId.IsEmpty())
{
// Unauthenticated requests and API keys carry no user, so there is nothing to
// check: leave the requirement unsatisfied and let the request be challenged.
return Task.CompletedTask;
}
var user = _userManager.GetUserById(userId);
if (user is null)
{
@@ -1,12 +1,17 @@
using System;
using System.Threading;
using System.Threading.Tasks;
using Jellyfin.Database.Implementations.Entities;
using MediaBrowser.Controller.Entities;
using MediaBrowser.Controller.Library;
using MediaBrowser.Controller.Session;
using MediaBrowser.Controller.SyncPlay.PlaybackRequests;
using MediaBrowser.Controller.SyncPlay.Requests;
using MediaBrowser.Model.SyncPlay;
using Microsoft.Extensions.Logging.Abstractions;
using Moq;
using Xunit;
using SyncPlayGroup = Emby.Server.Implementations.SyncPlay.Group;
using SyncPlayManager = Emby.Server.Implementations.SyncPlay.SyncPlayManager;
namespace Jellyfin.Server.Implementations.Tests.SyncPlay;
@@ -55,11 +60,33 @@ public class SyncPlayManagerTests
Assert.False(harness.Manager.IsUserActive(harness.User.Id));
}
[Fact]
public async Task HandleRequest_GroupWaitsForAMemberThatNeverReportsReady_RecoversOnItsOwn()
{
var harness = new ManagerHarness(groupWaitTimeout: 200);
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);
// Starting playback puts the group behind the ready barrier.
harness.Manager.HandleRequest(
harness.Session,
new PlayGroupRequest(new[] { Guid.NewGuid() }, 0, 0),
CancellationToken.None);
Assert.Equal(GroupStateType.Waiting, harness.Manager.GetGroup(harness.Session, info.GroupId).State);
// Neither session ever reports ready, so the group has to come out of the wait by itself.
Assert.Equal(
GroupStateType.Playing,
await harness.WaitForState(harness.Session, info.GroupId, GroupStateType.Playing));
}
private sealed class ManagerHarness
{
private readonly Mock<ISessionManager> _sessionManager = new();
public ManagerHarness()
public ManagerHarness(long? groupWaitTimeout = null)
{
var userManager = new Mock<IUserManager>();
var libraryManager = new Mock<ILibraryManager>();
@@ -67,11 +94,26 @@ public class SyncPlayManagerTests
User = new User("tester", "auth-provider", "pwdreset-provider");
userManager.Setup(m => m.GetUserById(It.IsAny<Guid>())).Returns(User);
var item = new Mock<BaseItem>();
item.Setup(i => i.IsVisibleStandalone(It.IsAny<User>())).Returns(true);
item.Object.RunTimeTicks = TimeSpan.FromHours(2).Ticks;
libraryManager.Setup(m => m.GetItemById(It.IsAny<Guid>())).Returns(item.Object);
_sessionManager
.Setup(m => m.SendSyncPlayCommand(It.IsAny<string>(), It.IsAny<SendCommand>(), It.IsAny<CancellationToken>()))
.Returns(Task.CompletedTask);
_sessionManager
.Setup(m => m.SendSyncPlayGroupUpdate(It.IsAny<string>(), It.IsAny<GroupUpdate<GroupStateUpdate>>(), It.IsAny<CancellationToken>()))
.Returns(Task.CompletedTask);
Manager = new SyncPlayManager(
NullLoggerFactory.Instance,
userManager.Object,
_sessionManager.Object,
libraryManager.Object);
libraryManager.Object)
{
GroupWaitTimeout = groupWaitTimeout ?? SyncPlayGroup.DefaultGroupWaitTimeout
};
Session = CreateSession("session-1");
}
@@ -91,5 +133,17 @@ public class SyncPlayManagerTests
UserName = User.Username
};
}
public async Task<GroupStateType> WaitForState(SessionInfo session, Guid groupId, GroupStateType expected)
{
var deadline = DateTime.UtcNow.AddSeconds(10);
GroupStateType state;
while ((state = Manager.GetGroup(session, groupId).State) != expected && DateTime.UtcNow < deadline)
{
await Task.Delay(20, TestContext.Current.CancellationToken);
}
return state;
}
}
}
@@ -204,9 +204,131 @@ public class WaitingGroupStateTests
Assert.InRange(group.LastActivity - before, TimeSpan.Zero, TimeSpan.FromMinutes(1));
}
[Fact]
public async Task SessionJoined_JoinerNeverReportsReady_GroupResumesWithoutIt()
{
var harness = new GroupHarness(groupWaitTimeout: 200);
var group = harness.Group;
group.PositionTicks = TimeSpan.FromMinutes(5).Ticks;
group.LastActivity = DateTime.UtcNow;
group.SetState(new PlayingGroupState(NullLoggerFactory.Instance));
// A session joins while the group is playing: the group pauses and waits for it.
var joiner = harness.NewSession("joiner");
group.SessionJoin(joiner, new JoinGroupRequest(group.GroupId), CancellationToken.None);
Assert.Equal(GroupStateType.Waiting, group.GetInfo().State);
// The joiner's player aborts and never reports ready. Without a bounded wait the whole
// group stays paused forever.
await harness.WaitForState(GroupStateType.Playing);
// Late buffer reports from the session that missed the deadline must not drag the group
// back into waiting.
group.HandleRequest(
joiner,
new BufferGroupRequest(DateTime.UtcNow, 0, false, harness.PlaylistItemId),
CancellationToken.None);
Assert.Equal(GroupStateType.Playing, group.GetInfo().State);
}
[Fact]
public async Task SessionJoined_GroupWasPaused_TimeoutLeavesTheGroupPaused()
{
var harness = new GroupHarness(groupWaitTimeout: 200);
var group = harness.Group;
group.PositionTicks = TimeSpan.FromMinutes(5).Ticks;
// The group has been sitting paused for a while before anyone joins.
group.LastActivity = DateTime.UtcNow.AddMinutes(-2);
group.SetState(new PausedGroupState(NullLoggerFactory.Instance));
var joiner = harness.NewSession("joiner");
group.SessionJoin(joiner, new JoinGroupRequest(group.GroupId), CancellationToken.None);
Assert.Equal(GroupStateType.Waiting, group.GetInfo().State);
// A group that was paused must not start playing because a member failed to report ready.
await harness.WaitForState(GroupStateType.Paused);
// Giving up on the joiner must not move the playback position of an already paused group.
Assert.Equal(TimeSpan.FromMinutes(5).Ticks, group.PositionTicks);
// Every member has to be told the group is no longer waiting.
var recipients = harness.StateUpdates
.Where(update => update.Update.State == GroupStateType.Paused)
.Select(update => update.SessionId)
.ToList();
Assert.Contains(harness.First.Id, recipients);
Assert.Contains(harness.Second.Id, recipients);
Assert.Contains(joiner.Id, recipients);
}
[Fact]
public async Task Ready_ReportedBeforeTheDeadline_GroupDoesNotGiveUpOnAnyone()
{
var harness = new GroupHarness(groupWaitTimeout: 200);
var group = harness.Group;
group.PositionTicks = TimeSpan.FromMinutes(5).Ticks;
group.LastActivity = DateTime.UtcNow;
group.SetState(new PlayingGroupState(NullLoggerFactory.Instance));
var joiner = harness.NewSession("joiner");
group.SessionJoin(joiner, new JoinGroupRequest(group.GroupId), CancellationToken.None);
Assert.Equal(GroupStateType.Waiting, group.GetInfo().State);
group.HandleRequest(
joiner,
new ReadyGroupRequest(DateTime.UtcNow, group.PositionTicks, true, harness.PlaylistItemId),
CancellationToken.None);
// Everyone reported ready, so no deadline is left to trip and force a spurious unpause.
Assert.Equal(GroupStateType.Playing, group.GetInfo().State);
Assert.Null(group.GroupWaitDeadline);
var until = DateTime.UtcNow.AddMilliseconds(3 * 200);
while (DateTime.UtcNow < until)
{
harness.PumpGroupWaitTimeout();
await Task.Delay(20, TestContext.Current.CancellationToken);
}
Assert.Equal(GroupStateType.Playing, group.GetInfo().State);
}
[Fact]
public async Task SetPlaylistItem_AfterATimeout_GroupWaitsForEveryoneAgain()
{
var harness = new GroupHarness(groupWaitTimeout: 200);
var group = harness.Group;
group.LastActivity = DateTime.UtcNow;
group.SetState(new PlayingGroupState(NullLoggerFactory.Instance));
var joiner = harness.NewSession("joiner");
group.SessionJoin(joiner, new JoinGroupRequest(group.GroupId), CancellationToken.None);
await harness.WaitForState(GroupStateType.Playing);
// Giving up on a session lasts only until the group changes what it is playing.
group.HandleRequest(
harness.First,
new SetPlaylistItemGroupRequest(harness.PlaylistItemId),
CancellationToken.None);
Assert.Equal(GroupStateType.Waiting, group.GetInfo().State);
Assert.NotNull(group.GroupWaitDeadline);
}
private sealed class GroupHarness
{
public GroupHarness()
private readonly ISessionManager _sessionManager;
private readonly Guid _userId;
public GroupHarness(long? groupWaitTimeout = null)
{
var userManager = new Mock<IUserManager>();
var sessionManager = new Mock<ISessionManager>();
@@ -227,26 +349,23 @@ public class WaitingGroupStateTests
sessionManager
.Setup(m => m.SendSyncPlayGroupUpdate(It.IsAny<string>(), It.IsAny<GroupUpdate<GroupStateUpdate>>(), It.IsAny<CancellationToken>()))
.Callback((string sessionId, GroupUpdate<GroupStateUpdate> update, CancellationToken _) => StateUpdates.Add((sessionId, update.Data)))
.Returns(Task.CompletedTask);
Group = new SyncPlayGroup(
NullLoggerFactory.Instance,
userManager.Object,
sessionManager.Object,
libraryManager.Object);
libraryManager.Object)
{
GroupWaitTimeout = groupWaitTimeout ?? SyncPlayGroup.DefaultGroupWaitTimeout
};
First = new SessionInfo(sessionManager.Object, NullLogger.Instance)
{
Id = "first",
UserId = user.Id,
UserName = "first"
};
Second = new SessionInfo(sessionManager.Object, NullLogger.Instance)
{
Id = "second",
UserId = user.Id,
UserName = "second"
};
_sessionManager = sessionManager.Object;
_userId = user.Id;
First = NewSession("first");
Second = NewSession("second");
Group.CreateGroup(First, new NewGroupRequest("group"), CancellationToken.None);
Group.SessionJoin(Second, new JoinGroupRequest(Group.GroupId), CancellationToken.None);
@@ -256,6 +375,8 @@ public class WaitingGroupStateTests
public SyncPlayGroup Group { get; }
public List<(string SessionId, GroupStateUpdate Update)> StateUpdates { get; } = new();
public SessionInfo First { get; }
public SessionInfo Second { get; }
@@ -263,5 +384,39 @@ public class WaitingGroupStateTests
public Guid PlaylistItemId { get; }
public List<SendCommand> Commands { get; } = new List<SendCommand>();
// Mirrors the sweep SyncPlayManager runs on a timer.
public void PumpGroupWaitTimeout()
{
var group = Group;
// Group lock required as Group is not thread-safe.
lock (group)
{
group.HandleGroupWaitTimeout(CancellationToken.None);
}
}
public async Task WaitForState(GroupStateType expected)
{
var deadline = DateTime.UtcNow.AddSeconds(10);
while (Group.GetInfo().State != expected && DateTime.UtcNow < deadline)
{
PumpGroupWaitTimeout();
await Task.Delay(20, TestContext.Current.CancellationToken);
}
Assert.Equal(expected, Group.GetInfo().State);
}
public SessionInfo NewSession(string id)
{
return new SessionInfo(_sessionManager, NullLogger.Instance)
{
Id = id,
UserId = _userId,
UserName = id
};
}
}
}
@@ -0,0 +1,36 @@
using System.Net;
using System.Threading.Tasks;
using Xunit;
namespace Jellyfin.Server.Integration.Tests.Controllers;
public sealed class SyncPlayControllerTests : IClassFixture<JellyfinApplicationFactory>
{
private readonly JellyfinApplicationFactory _factory;
public SyncPlayControllerTests(JellyfinApplicationFactory factory)
{
_factory = factory;
}
[Fact]
public async Task GetGroups_Unauthorized_ReturnsUnauthorized()
{
var client = _factory.CreateClient();
var response = await client.GetAsync("/SyncPlay/List", TestContext.Current.CancellationToken);
Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode);
}
[Fact]
public async Task GetGroups_InvalidToken_ReturnsUnauthorized()
{
var client = _factory.CreateClient();
client.DefaultRequestHeaders.AddAuthHeader("invalid-token");
var response = await client.GetAsync("/SyncPlay/List", TestContext.Current.CancellationToken);
Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode);
}
}