Fix events.onexit callbacks accessing a disposed core and crashing#4638
Fix events.onexit callbacks accessing a disposed core and crashing#4638
events.onexit callbacks accessing a disposed core and crashing#4638Conversation
SuuperW
left a comment
There was a problem hiding this comment.
This is mostly doing the same thing that my PR does, but it is making an exception for Lua rather than a generic solution for any tools that may end up with a similar issue. As I've stated, I prefer the generic solution. Do you have a reason to prefer making an exception for Lua? I don't think this one is meaningfully simpler.
But there is one issue with the way you've done this: If a Lua script causes the restart then exit events will get called without the scripts being stopped (or re-started). This will leave scripts in an undefined state. You need to check LuaImp.IsRebootingCore.
doesn't interfere with running onexit callbacks when individual scripts are stopped
Did mine? I'm not aware of any issues with my PR.
| public void CallStateSaveCallbacks(string userFriendlyStateName) | ||
| => LuaImp.CallSaveStateEvent(userFriendlyStateName); | ||
|
|
||
| public void CallScriptExitCallbacks() |
There was a problem hiding this comment.
I think the purpose of this needs to be documented. Calling the exit event without stopping the script is a weird thing to do, and that should be noted along with why it's done here.
Alternatively, you could do what I did and actually stop the scripts (renaming the method accordingly). Just keep a list of which ones were running so they can be started again in restart.
| if (saveMovieResult == TryAgainResult.Canceled) return false; | ||
| } | ||
|
|
||
| if (Tools.Has<LuaConsole>()) Tools.LuaConsole.CallScriptExitCallbacks(); // important to do this here with the stale `IEmulator`, see https://github.com/TASEmulators/BizHawk/issues/4629 |
There was a problem hiding this comment.
The emulator isn't stale yet, it is still current.
Dead simple, and doesn't interfere with running
onexitcallbacks when individual scripts are stopped.