Merge pull request #10790 from peppy/fix-spectator-replay-thread-safety

Fix frames potentially getting added to spectator replay in wrong format
This commit is contained in:
Dan Balasescu 2020-11-11 15:02:28 +09:00 committed by GitHub
commit acee24e4fd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -3,6 +3,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Diagnostics;
using System.Linq; using System.Linq;
using JetBrains.Annotations; using JetBrains.Annotations;
using osu.Framework.Allocation; using osu.Framework.Allocation;
@ -61,7 +62,9 @@ namespace osu.Game.Screens.Play
[Resolved] [Resolved]
private RulesetStore rulesets { get; set; } private RulesetStore rulesets { get; set; }
private Replay replay; private Score score;
private readonly object scoreLock = new object();
private Container beatmapPanelContainer; private Container beatmapPanelContainer;
@ -198,14 +201,22 @@ namespace osu.Game.Screens.Play
private void userSentFrames(int userId, FrameDataBundle data) private void userSentFrames(int userId, FrameDataBundle data)
{ {
// this is not scheduled as it handles propagation of frames even when in a child screen (at which point we are not alive).
// probably not the safest way to handle this.
if (userId != targetUser.Id) if (userId != targetUser.Id)
return; return;
lock (scoreLock)
{
// this should never happen as the server sends the user's state on watching, // this should never happen as the server sends the user's state on watching,
// but is here as a safety measure. // but is here as a safety measure.
if (replay == null) if (score == null)
return; return;
// rulesetInstance should be guaranteed to be in sync with the score via scoreLock.
Debug.Assert(rulesetInstance != null && rulesetInstance.RulesetInfo.Equals(score.ScoreInfo.Ruleset));
foreach (var frame in data.Frames) foreach (var frame in data.Frames)
{ {
IConvertibleReplayFrame convertibleFrame = rulesetInstance.CreateConvertibleReplayFrame(); IConvertibleReplayFrame convertibleFrame = rulesetInstance.CreateConvertibleReplayFrame();
@ -214,7 +225,8 @@ namespace osu.Game.Screens.Play
var convertedFrame = (ReplayFrame)convertibleFrame; var convertedFrame = (ReplayFrame)convertibleFrame;
convertedFrame.Time = frame.Time; convertedFrame.Time = frame.Time;
replay.Frames.Add(convertedFrame); score.Replay.Frames.Add(convertedFrame);
}
} }
} }
@ -247,10 +259,13 @@ namespace osu.Game.Screens.Play
if (userId != targetUser.Id) if (userId != targetUser.Id)
return; return;
if (replay != null) lock (scoreLock)
{ {
replay.HasReceivedAllFrames = true; if (score != null)
replay = null; {
score.Replay.HasReceivedAllFrames = true;
score = null;
}
} }
Schedule(clearDisplay); Schedule(clearDisplay);
@ -283,14 +298,18 @@ namespace osu.Game.Screens.Play
return; return;
} }
replay ??= new Replay { HasReceivedAllFrames = false }; lock (scoreLock)
{
var scoreInfo = new ScoreInfo score = new Score
{
ScoreInfo = new ScoreInfo
{ {
Beatmap = resolvedBeatmap, Beatmap = resolvedBeatmap,
User = targetUser, User = targetUser,
Mods = state.Mods.Select(m => m.ToMod(resolvedRuleset)).ToArray(), Mods = state.Mods.Select(m => m.ToMod(resolvedRuleset)).ToArray(),
Ruleset = resolvedRuleset.RulesetInfo, Ruleset = resolvedRuleset.RulesetInfo,
},
Replay = new Replay { HasReceivedAllFrames = false },
}; };
ruleset.Value = resolvedRuleset.RulesetInfo; ruleset.Value = resolvedRuleset.RulesetInfo;
@ -299,11 +318,8 @@ namespace osu.Game.Screens.Play
beatmap.Value = beatmaps.GetWorkingBeatmap(resolvedBeatmap); beatmap.Value = beatmaps.GetWorkingBeatmap(resolvedBeatmap);
watchButton.Enabled.Value = true; watchButton.Enabled.Value = true;
this.Push(new SpectatorPlayerLoader(new Score this.Push(new SpectatorPlayerLoader(score));
{ }
ScoreInfo = scoreInfo,
Replay = replay,
}));
} }
private void showBeatmapPanel(SpectatorState state) private void showBeatmapPanel(SpectatorState state)