Skip to content

Commit 8096b22

Browse files
committed
Stop tools before rebooting. Fixes crash if a Lua script's exit handler did a memory read or write.
1 parent c8af415 commit 8096b22

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
@@ -3942,6 +3942,8 @@ private bool CloseGame(bool clearSram = false)
39423942
if (saveMovieResult == TryAgainResult.Canceled) return false;
39433943
}
39443944

3945+
Tools.Stop();
3946+
39453947
if (clearSram)
39463948
{
39473949
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
@@ -263,6 +263,18 @@ public string FromSlot
263263
protected override void UpdateAfter() => Update(fast: false);
264264
protected override void FastUpdateAfter() => Update(fast: true);
265265

266+
public override void Stop()
267+
{
268+
if (_isBotting)
269+
{
270+
StopBot();
271+
}
272+
else if (_replayMode)
273+
{
274+
FinishReplay();
275+
}
276+
}
277+
266278
public override void Restart()
267279
{
268280
_ = StatableCore!; // otherwise unused due to loadstating via MainForm; however this service is very much required so the property needs to be present
@@ -275,16 +287,6 @@ public override void Restart()
275287
_dataSize = 1;
276288
}
277289

278-
if (_isBotting)
279-
{
280-
StopBot();
281-
}
282-
else if (_replayMode)
283-
{
284-
FinishReplay();
285-
}
286-
287-
288290
if (_lastRom != MainForm.CurrentlyOpenRom)
289291
{
290292
_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
@@ -156,6 +156,7 @@ public LuaConsole()
156156
}
157157

158158
private LuaLibraries LuaImp;
159+
private List<LuaFile> _scriptsToRestart = new();
159160

160161
private IEnumerable<LuaFile> SelectedItems => LuaListView.SelectedRows.Select(index => LuaImp.ScriptList[index]);
161162

@@ -186,10 +187,22 @@ private void BranchesMarkersSplit_SplitterMoved(object sender, SplitterEventArgs
186187
Settings.SplitDistance = splitContainer1.SplitterDistance;
187188
}
188189

189-
public override void Restart()
190+
public override void Stop()
190191
{
191-
List<LuaFile> runningScripts = new();
192+
if (LuaImp is not null && !LuaImp.IsRebootingCore)
193+
{
194+
_scriptsToRestart = LuaImp.ScriptList.Where(lf => lf.Enabled).ToList();
192195

196+
// we don't use _scriptsToRestart here as the other scripts need to be stopped too
197+
foreach (var file in LuaImp.ScriptList)
198+
{
199+
DisableLuaScript(file);
200+
}
201+
}
202+
}
203+
204+
public override void Restart()
205+
{
193206
ApiContainer apiContainer = ApiManager.RestartLua(
194207
Emulator.ServiceProvider,
195208
WriteToOutputWindow,
@@ -203,23 +216,11 @@ public override void Restart()
203216
Game,
204217
DialogController);
205218

206-
// Things we need to do with the existing LuaImp before we can make a new one
207-
if (LuaImp is not null)
219+
if (LuaImp is not null && LuaImp.IsRebootingCore)
208220
{
209-
if (LuaImp.IsRebootingCore)
210-
{
211-
// Even if the lua console is self-rebooting from client.reboot_core() we still want to re-inject dependencies
212-
LuaImp.Restart(Emulator.ServiceProvider, Config, apiContainer);
213-
return;
214-
}
215-
216-
runningScripts = LuaImp.ScriptList.Where(lf => lf.Enabled).ToList();
217-
218-
// we don't use runningScripts here as the other scripts need to be stopped too
219-
foreach (var file in LuaImp.ScriptList)
220-
{
221-
DisableLuaScript(file);
222-
}
221+
// Even if the lua console is self-rebooting from client.reboot_core() we still want to re-inject dependencies
222+
LuaImp.Restart(Emulator.ServiceProvider, Config, apiContainer);
223+
return;
223224
}
224225

225226
LuaFileList newScripts = new(LuaImp?.ScriptList, onChanged: SessionChangedCallback);
@@ -240,7 +241,7 @@ public override void Restart()
240241
.Select(static f => $"{f.Library}.{f.Name}")
241242
.ToArray());
242243

243-
foreach (var file in runningScripts)
244+
foreach (var file in _scriptsToRestart)
244245
{
245246
try
246247
{

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
@@ -34,6 +34,7 @@ public class ToolManager
3434
// For instance, add an IToolForm property called UsesCheats, so that a UpdateCheatRelatedTools() method can update all tools of this type
3535
// Also a UsesRam, and similar method
3636
private readonly List<IToolForm> _tools = new List<IToolForm>();
37+
private readonly List<IToolForm> _toolsToRestart = new();
3738

3839
/// <summary>
3940
/// Initializes a new instance of the <see cref="ToolManager"/> class.
@@ -589,6 +590,15 @@ public void GeneralUpdateActiveExtTools()
589590
}
590591
}
591592

593+
public void Stop()
594+
{
595+
foreach (IToolForm tool in _tools.Where(static t => t.IsActive))
596+
{
597+
_toolsToRestart.Add(tool);
598+
tool.Stop();
599+
}
600+
}
601+
592602
public void Restart(Config config, IEmulator emulator, IGameInfo game)
593603
{
594604
_config = config;
@@ -604,7 +614,7 @@ public void Restart(Config config, IEmulator emulator, IGameInfo game)
604614
if (ServiceInjector.UpdateServices(_emulator.ServiceProvider, tool)
605615
&& (tool is not IExternalToolForm || ApiInjector.UpdateApis(GetOrInitApiProvider, tool)))
606616
{
607-
if (tool.IsActive) tool.Restart();
617+
if (_toolsToRestart.Contains(tool)) tool.Restart();
608618
}
609619
else
610620
{
@@ -618,6 +628,8 @@ public void Restart(Config config, IEmulator emulator, IGameInfo game)
618628
tool.Close();
619629
_tools.Remove(tool);
620630
}
631+
632+
_toolsToRestart.Clear();
621633
}
622634

623635
/// <summary>

0 commit comments

Comments
 (0)