Godot: GDScript coroutine resumption crashes after script hot reload

[timoschwarzer]

Godot version:
3.1-alpha4

OS/device including version:
Ubuntu 18.10

Issue description:
The engine crashes with Internal script error! - opcode #0 (report please) when resuming a coroutine after having hot-reloaded a script. If you have coroutines running, you can't really use the hot-reload feature because the engine will always crash when any coroutine is about to resume.

Stack trace from debugger:

E 0:00:10:0191   Condition ' (ip + 5) > _code_size ' is true. Breaking..:
  <C Source>     modules/gdscript/gdscript_function.cpp:436 @ call()
  <Stack Trace>  Scene.gd:9 @ _init()

Steps to reproduce:

1. Start a coroutine in GDScript (e.g. yield(get_tree().create_timer(10.0), 'timeout')
2. Change a script while the coroutine is running and save to hot-reload
3. Wait for the coroutine to resume and crash the engine

Minimal reproduction project:
coroutine-crash.zip

[timoschwarzer]

Just a status update: Still reproducable with 3.1-stable

[qarmin]

I have this invalid read when running project

==25959==ERROR: AddressSanitizer: heap-use-after-free on address 0x61400001acb0 at pc 0x000001948de3 bp 0x7ffc6dd18500 sp 0x7ffc6dd184f0
READ of size 8 at 0x61400001acb0 thread T0
    #0 0x1948de2 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_function.cpp:264
    #1 0x197e05d in GDScriptFunctionState::resume(Variant const&) modules/gdscript/gdscript_function.cpp:1847
    #2 0x197ca78 in GDScriptFunctionState::_signal_callback(Variant const**, int, Variant::CallError&) modules/gdscript/gdscript_function.cpp:1817
    #3 0x19a118c in MethodBindVarArg<GDScriptFunctionState>::call(Object*, Variant const**, int, Variant::CallError&) (/home/rafal/Pulpit/mojgodot/bin/godot.x11.tools.64s+0x19a118c)
    #4 0xcd59ebc in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:921
    #5 0xcd626d6 in Object::emit_signal(StringName const&, Variant const**, int) core/object.cpp:1218
    #6 0xcd642ab in Object::emit_signal(StringName const&, Variant const&, Variant const&, Variant const&, Variant const&, Variant const&) core/object.cpp:1274
    #7 0x856da8e in SceneTree::idle(float) scene/main/scene_tree.cpp:566
    #8 0x1561c1c in Main::iteration() main/main.cpp:1980
    #9 0x147964e in OS_X11::run() platform/x11/os_x11.cpp:3260
    #10 0x13ff2e9 in main platform/x11/godot_x11.cpp:56
    #11 0x7f59081fc1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)
    #12 0x13fef0d in _start (/home/rafal/Pulpit/mojgodot/bin/godot.x11.tools.64s+0x13fef0d)

0x61400001acb0 is located 112 bytes inside of 448-byte region [0x61400001ac40,0x61400001ae00)
freed by thread T0 here:
    #0 0x7f59097206ef in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0x10d6ef)
    #1 0xd1d918f in Memory::free_static(void*, bool) core/os/memory.cpp:178
    #2 0x1816835 in void memdelete<GDScriptFunction>(GDScriptFunction*) core/os/memory.h:119
    #3 0x1897985 in GDScriptCompiler::_parse_class_level(GDScript*, GDScriptParser::ClassNode const*, bool) modules/gdscript/gdscript_compiler.cpp:1843
    #4 0x18a0d37 in GDScriptCompiler::compile(GDScriptParser const*, GDScript*, bool) modules/gdscript/gdscript_compiler.cpp:2154
    #5 0x17c601e in GDScript::reload(bool) modules/gdscript/gdscript.cpp:582
    #6 0x17e3dc4 in GDScriptLanguage::reload_all_scripts() modules/gdscript/gdscript.cpp:1592
    #7 0xb4d64fa in ScriptDebuggerRemote::idle_poll() scene/debugger/script_debugger_remote.cpp:1014
    #8 0x15634aa in Main::iteration() main/main.cpp:2015
    #9 0x147964e in OS_X11::run() platform/x11/os_x11.cpp:3260
    #10 0x13ff2e9 in main platform/x11/godot_x11.cpp:56
    #11 0x7f59081fc1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

previously allocated by thread T0 here:
    #0 0x7f5909720ae8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
    #1 0xd1d8321 in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:82
    #2 0xd1d8220 in operator new(unsigned long, char const*) core/os/memory.cpp:42
    #3 0x1891e9f in GDScriptCompiler::_parse_function(GDScript*, GDScriptParser::ClassNode const*, GDScriptParser::FunctionNode const*, bool) modules/gdscript/gdscript_compiler.cpp:1672
    #4 0x189d97d in GDScriptCompiler::_parse_class_blocks(GDScript*, GDScriptParser::ClassNode const*, bool) modules/gdscript/gdscript_compiler.cpp:2017
    #5 0x18a0e69 in GDScriptCompiler::compile(GDScriptParser const*, GDScript*, bool) modules/gdscript/gdscript_compiler.cpp:2159
    #6 0x17c601e in GDScript::reload(bool) modules/gdscript/gdscript.cpp:582
    #7 0x17f7d34 in ResourceFormatLoaderGDScript::load(String const&, String const&, Error*) modules/gdscript/gdscript.cpp:2190
    #8 0xd5e9d0d in ResourceLoader::_load(String const&, String const&, String const&, bool, Error*) core/io/resource_loader.cpp:270
    #9 0xd5ec159 in ResourceLoader::load(String const&, String const&, bool, Error*) core/io/resource_loader.cpp:396
    #10 0xad9aaf5 in ResourceInteractiveLoaderText::poll() scene/resources/resource_format_text.cpp:433
    #11 0xd5e4d0c in ResourceFormatLoader::load(String const&, String const&, Error*) core/io/resource_loader.cpp:197
    #12 0xd5e9d0d in ResourceLoader::_load(String const&, String const&, String const&, bool, Error*) core/io/resource_loader.cpp:270
    #13 0xd5ec159 in ResourceLoader::load(String const&, String const&, bool, Error*) core/io/resource_loader.cpp:396
    #14 0x155b175 in Main::start() main/main.cpp:1812
    #15 0x13ff219 in main platform/x11/godot_x11.cpp:55
    #16 0x7f59081fc1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

[vnen]

Well, it makes sense that this happens. We need a way to either invalidate the pending function states (so they are not resumed) or update their instance reference (which might be a unsafe since the function code might change).

[timoschwarzer]

@vnen I'd vote for the latter. I usually work a lot with coroutines in Godot, so it's likely that the game breaks if a coroutine does not continue. (e.g.: Show an overlay, wait for a signal, hide the overlay. If we'd discard the coroutine here while waiting for the signal, the overlay would be shown permanently).
In which way would it be unsafe because the function code changes? Isn't that what hot-reloading does anyways?

[bojidar-bg]

@timoschwarzer The main problem is "shifting" the point in which the function is resumed in cases it was modified.

[timoschwarzer]

Ah I see. Would it be easier to see if code has changed and only discard those routines?

[vnen]

Not really critical since this only affect the editor.

[timoschwarzer]

@vnen I disagree. This bug prevents users from successfully using script hot reloading at all when making heavy use of coroutines.

[vnen]

@timoschwarzer I still don't see that as "critical". It's inconvenient. My point is more is that there's no simple fix and should be done after 3.2 is released so we don't risk introducing regressions this close to release.

[SolarLune]

Hello, I'm running into this as well when using yield(get_tree(), "idle_frame") to pause the _ready() function of an object for a frame with Sync Script Changes on. Note that I don't actually change the script and hotload it; it just crashes with Opcode #0 error, as the OP mentions. I'm using Godot 3.2 official on Linux (Solus OS, to be exact).

Just wanted to give a +1 for this issue and show another case for how it crops up.

EDIT: Also, seems like this and #30193 are the same issues?

[tavurth]

Still facing this, and it seems to be an issue even when the files I'm hot reloading don't have coroutines.

I have some long running co-routines in other areas such as singletons, where it seems to break down.

Does Godot try to HMR the entire stack of all functions (singletons) upon every HMR request?

In webpack this is handled by allowing a user to opt-out of HMR functionality inside any given module, by using module.hot.accept.

It would be nice to have HMR as most of my code-base would function very well without any changes to these singletons where my data is arriving.