Clear MIDI masters properly to avoid replay freezes (#36809)

While trying to play a replay I noticed that the replay would freeze
when seeking in some cases. After some debugging, I discovered that two
MIDI renderers had each other as master, which caused an infinite loop
processing MIDI events.

I'm not entirely sure of the sequence of events that leads to this
during replay playback, but I did notice that MIDI render masters are
never set to null. This is in the best case just a memory leak, in the
worst case probably the source of the bug, so... I fixed that.

diff --git a/Content.Client/Instruments/InstrumentSystem.cs b/Content.Client/Instruments/InstrumentSystem.cs
index 5bdaa52359da1a2844d6a4855da75641a9a1f0af..abc3fa8210e94de0ccac426897689869110f8ba6 100644 (file)
--- a/Content.Client/Instruments/InstrumentSystem.cs
+++ b/Content.Client/Instruments/InstrumentSystem.cs
@@ -167,11 +167,14 @@ public sealed class InstrumentSystem : SharedInstrumentSystem
 
     private void UpdateRendererMaster(InstrumentComponent instrument)
     {
-        if (instrument.Renderer == null || instrument.Master == null)
+        if (instrument.Renderer == null)
             return;
 
-        if (!TryComp(instrument.Master, out InstrumentComponent? masterInstrument) || masterInstrument.Renderer == null)
+        if (instrument.Master == null || !TryComp(instrument.Master, out InstrumentComponent? masterInstrument) || masterInstrument.Renderer == null)
+        {
+            instrument.Renderer.Master = null;
             return;
+        }
 
         instrument.Renderer.Master = masterInstrument.Renderer;
     }
@@ -196,15 +199,16 @@ public sealed class InstrumentSystem : SharedInstrumentSystem
             return;
         }
 
-        instrument.Renderer?.SystemReset();
-        instrument.Renderer?.ClearAllEvents();
-
-        var renderer = instrument.Renderer;
+        if (instrument.Renderer is { } renderer)
+        {
+            renderer.Master = null;
+            renderer.SystemReset();
+            renderer.ClearAllEvents();
 
-        // We dispose of the synth two seconds from now to allow the last notes to stop from playing.
-        // Don't use timers bound to the entity in case it is getting deleted.
-        if (renderer != null)
+            // We dispose of the synth two seconds from now to allow the last notes to stop from playing.
+            // Don't use timers bound to the entity in case it is getting deleted.
             Timer.Spawn(2000, () => { renderer.Dispose(); });
+        }
 
         instrument.Renderer = null;
         instrument.MidiEventBuffer.Clear();