From b629cbac3e6993164c6c8a6fe41d709f3a5ffd33 Mon Sep 17 00:00:00 2001 From: Pieter-Jan Briers Date: Sat, 26 Apr 2025 08:42:16 +0200 Subject: [PATCH] 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. --- .../Instruments/InstrumentSystem.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/Content.Client/Instruments/InstrumentSystem.cs b/Content.Client/Instruments/InstrumentSystem.cs index 5bdaa52359..abc3fa8210 100644 --- 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(); -- 2.51.2