Skip to content

Commit 46af866

Browse files
committed
Stop tools before rebooting. Fixes crash if a Lua script's exit handler did a memory read or write.
1 parent 6223c9d commit 46af866

7 files changed

Lines changed: 60 additions & 31 deletions

File tree

src/BizHawk.Client.Common/tools/Interfaces/IToolForm.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ public interface IToolForm
2727
/// </summary>
2828
void UpdateValues(ToolFormUpdateType type);
2929

30+
/// <summary>
31+
/// Stop the tool, in preparation for a game or core change.
32+
/// A tool should expect that either <see cref="Restart"/> or <see cref="Close"/> will be called after this.
33+
/// </summary>
34+
void Stop();
35+
3036
/// <summary>
3137
/// Will be called anytime the dialog needs to be restarted, such as when a new ROM is loaded
3238
/// The tool implementing this needs to account for a Game and Core change

src/BizHawk.Client.EmuHawk/MainForm.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3936,6 +3936,8 @@ private bool CloseGame(bool clearSram = false)
39363936
if (saveMovieResult == TryAgainResult.Canceled) return false;
39373937
}
39383938

3939+
Tools.Stop();
3940+
39393941
if (clearSram)
39403942
{
39413943
var path = Config.PathEntries.SaveRamAbsolutePath(Game, MovieSession.Movie);

src/BizHawk.Client.EmuHawk/tools/BasicBot/BasicBot.cs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,18 @@ public string FromSlot
260260
protected override void UpdateAfter() => Update(fast: false);
261261
protected override void FastUpdateAfter() => Update(fast: true);
262262

263+
public override void Stop()
264+
{
265+
if (_isBotting)
266+
{
267+
StopBot();
268+
}
269+
else if (_replayMode)
270+
{
271+
FinishReplay();
272+
}
273+
}
274+
263275
public override void Restart()
264276
{
265277
_ = StatableCore!; // otherwise unused due to loadstating via MainForm; however this service is very much required so the property needs to be present
@@ -272,16 +284,6 @@ public override void Restart()
272284
_dataSize = 1;
273285
}
274286

275-
if (_isBotting)
276-
{
277-
StopBot();
278-
}
279-
else if (_replayMode)
280-
{
281-
FinishReplay();
282-
}
283-
284-
285287
if (_lastRom != MainForm.CurrentlyOpenRom)
286288
{
287289
_lastRom = MainForm.CurrentlyOpenRom;

src/BizHawk.Client.EmuHawk/tools/Debugger/GenericDebugger.IToolForm.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,14 @@ private void FullUpdate()
112112
BreakPointControl1.UpdateValues();
113113
}
114114

115+
public override void Stop()
116+
{
117+
DisengageDebugger();
118+
}
119+
115120
public override void Restart()
116121
{
117122
UpdateCapabilitiesProps();
118-
DisengageDebugger();
119123
EngageDebugger();
120124
}
121125

src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ public LuaConsole()
163163
}
164164

165165
private LuaLibraries LuaImp;
166+
private List<LuaFile> _scriptsToRestart = new();
166167

167168
private ConsoleLuaLibrary _consoleLib;
168169

@@ -198,10 +199,22 @@ private void BranchesMarkersSplit_SplitterMoved(object sender, SplitterEventArgs
198199
Settings.SplitDistance = splitContainer1.SplitterDistance;
199200
}
200201

201-
public override void Restart()
202+
public override void Stop()
202203
{
203-
List<LuaFile> runningScripts = new();
204+
if (LuaImp is not null && !LuaImp.IsRebootingCore)
205+
{
206+
_scriptsToRestart = LuaImp.ScriptList.Where(lf => lf.Enabled).ToList();
204207

208+
// we don't use _scriptsToRestart here, in case something is somehow partially active
209+
foreach (var file in LuaImp.ScriptList)
210+
{
211+
file.Stop();
212+
}
213+
}
214+
}
215+
216+
public override void Restart()
217+
{
205218
ApiContainer apiContainer = ApiManager.RestartLua(
206219
Emulator.ServiceProvider,
207220
WriteToOutputWindow,
@@ -215,23 +228,11 @@ public override void Restart()
215228
Game,
216229
DialogController);
217230

218-
// Things we need to do with the existing LuaImp before we can make a new one
219-
if (LuaImp is not null)
231+
if (LuaImp is not null && LuaImp.IsRebootingCore)
220232
{
221-
if (LuaImp.IsRebootingCore)
222-
{
223-
// Even if the lua console is self-rebooting from client.reboot_core() we still want to re-inject dependencies
224-
LuaImp.Restart(Emulator.ServiceProvider, Config, apiContainer);
225-
return;
226-
}
227-
228-
runningScripts = LuaImp.ScriptList.Where(lf => lf.Enabled).ToList();
229-
230-
// we don't use runningScripts here as the other scripts need to be stopped too
231-
foreach (var file in LuaImp.ScriptList)
232-
{
233-
file.Stop();
234-
}
233+
// Even if the lua console is self-rebooting from client.reboot_core() we still want to re-inject dependencies
234+
LuaImp.Restart(Emulator.ServiceProvider, Config, apiContainer);
235+
return;
235236
}
236237

237238
_openedFiles = new(_openedFiles, onChanged: SessionChangedCallback);
@@ -255,7 +256,7 @@ public override void Restart()
255256
.Select(static f => $"{f.Library}.{f.Name}")
256257
.ToArray());
257258

258-
foreach (var file in runningScripts)
259+
foreach (var file in _scriptsToRestart)
259260
{
260261
EnableLuaFile(file);
261262
}

src/BizHawk.Client.EmuHawk/tools/ToolFormBase.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ public class ToolFormBase : FormBase, IToolForm, IDialogParent
3131
public virtual bool IsActive => IsHandleCreated && !IsDisposed;
3232
public virtual bool IsLoaded => IsActive;
3333

34+
public virtual void Stop() {}
35+
3436
public virtual void Restart() {}
3537

3638
public void SetToolFormBaseProps(

src/BizHawk.Client.EmuHawk/tools/ToolManager.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ static ToolManager()
4141
// For instance, add an IToolForm property called UsesCheats, so that a UpdateCheatRelatedTools() method can update all tools of this type
4242
// Also a UsesRam, and similar method
4343
private readonly List<IToolForm> _tools = new List<IToolForm>();
44+
private readonly List<IToolForm> _toolsToRestart = new();
4445

4546
/// <summary>
4647
/// Initializes a new instance of the <see cref="ToolManager"/> class.
@@ -582,6 +583,15 @@ public void GeneralUpdateActiveExtTools()
582583
}
583584
}
584585

586+
public void Stop()
587+
{
588+
foreach (IToolForm tool in _tools.Where(static t => t.IsActive))
589+
{
590+
_toolsToRestart.Add(tool);
591+
tool.Stop();
592+
}
593+
}
594+
585595
public void Restart(Config config, IEmulator emulator, IGameInfo game)
586596
{
587597
_config = config;
@@ -597,7 +607,7 @@ public void Restart(Config config, IEmulator emulator, IGameInfo game)
597607
if (ServiceInjector.UpdateServices(_emulator.ServiceProvider, tool)
598608
&& (tool is not IExternalToolForm || ApiInjector.UpdateApis(GetOrInitApiProvider, tool)))
599609
{
600-
if (tool.IsActive) tool.Restart();
610+
if (_toolsToRestart.Contains(tool)) tool.Restart();
601611
}
602612
else
603613
{
@@ -611,6 +621,8 @@ public void Restart(Config config, IEmulator emulator, IGameInfo game)
611621
tool.Close();
612622
_tools.Remove(tool);
613623
}
624+
625+
_toolsToRestart.Clear();
614626
}
615627

616628
/// <summary>

0 commit comments

Comments
 (0)