GDScript: Cancel suspended functions when reloading a script

[HolonProduction]

Note

Just because I'm in the GDScript team doesn't give credibility to this PR. It's my first time touching anything runtime related.

Related issues (not sure whether I should mark as closed, since the optimal solution would probably be to only cancel changed functions and replace the function pointer for others, (in the case of a hot reload that is triggered from saving in editor. Not from calling reload manually without any parameters)):
#86362
#92034
#100750

Before this, when reloading a script functions that were suspended using await would continue running after the reload. After the reload the memory location of GDScript functions might have changed, in those cases the VM would start executing the memory were the function was located before the reload, which would lead to a lot of invalid op code errors and sometimes crashes.

Now when reloading we cancel all pending functions. This makes the behavior in those cases consistent.

[HolonProduction]

Not sure what to do about the sanitizer thing. The leak is also present on master, there is just no test case that triggers it.

[Maran23]

Probably also fixes my issue: #97053. There also some other issues that sounds very much the same, so we should probably evaluate those as well after the merged.
Will test as soon as the build is green.

I expect that the whole script is reloaded, since I changed and saved it. Therefore, it need to be fully recompiled.
So at least for me, your proposed fix is fine and expected.

[Hilderin]

Thanks for looking into this issue.

I tested the MRP from #100750 with this PR and the error message is not there anymore, but now, this warning is always printed:
WARNING: modules\gdscript\gdscript.cpp:1638 - Canceling suspended execution of "refresh_filesystem" due to a script reload.

I'm not an expert, but cancelling awaits on script reload sounds a good idea! The problem with this solution for #100750 is that the script is reloaded for no valid reason, cancelling the refresh_filesystem method, resulting in never printing EditorPlugin.res_filesystem.update_file(filepath).

Finally, because of your added warning (thanks), that the issue from #100750 was caused because the plugin script was loaded twice on startup if the same script is used in the plugin ready and has an autoload. Autoloads are created really early, before the first scan, which causes the first load. When the plugin enabled,, the script is loaded with CACHE_MODE_IGNORE, causing the script to reload while awais are running.

First load:

Second load:

I changed the cache mode when enabling plugins on editor startup and that fixes #100750. I'll create a separate PR just for that fix because, the way I see it, the double reloading is a separate issue.

[HolonProduction]

Yep I was aware that this doesn't make the code from #100750 work, but it fails consistently with this PR, which is preferable to crashes. Fixing the actual reload thing is a separate thing, but as I stated over there I'd be careful with changing this cache mode thing. It was introduced in #73776 to fix some bug, so we need to carefully test whether removing it would reintroduce the original bug (which seems far worse, given that #100750 can be somewhat worked around by waiting for a few cycles before calling to the editor plugin script).

[Ivorforce]

The leak may be caused by the same reference cycles I outlined in this issue: #101830

It's quite difficult to check though, so I can't guarantee it's the case. But if it's triggered with master code before the PR, I think it's fair to conclude this PR doesn't cause it.

[HolonProduction]

I am able to get the exact same leaks on master, when copying over the test case and removing the finished.emit() line. Given that this doesn't seem related to this PR, I disabled the test case for now. We can try enabling it again once #101830 is addressed or other leak related fixes get merged.