Skip to content

GDScript: Fix crash when DEV_ENABLED#117346

Closed
stuartcarnie wants to merge 1 commit into
godotengine:masterfrom
stuartcarnie:gdscript_vm_crash
Closed

GDScript: Fix crash when DEV_ENABLED#117346
stuartcarnie wants to merge 1 commit into
godotengine:masterfrom
stuartcarnie:gdscript_vm_crash

Conversation

@stuartcarnie
Copy link
Copy Markdown
Contributor

I discovered this whilst testing the MR) for #117339, which was crashing with DEV_ENABLED.

Fix double-free of GDScript coroutine stack when re-awaiting

Fixes a crash caused by a double-free of stack Variants in the GDScript VM when a resumed coroutine hits another await.

Root Cause

When GDScriptFunction::call() is invoked with a saved state (p_state != nullptr) and the function suspends again (awaited = true), the cleanup code at the end of call() destroys the stack Variants in the old state's buffer:

if (!p_state || awaited) {
    // ...
    for (int i = FIXED_ADDRESSES_MAX; i < _stack_size; i++) {
        stack[i].~Variant();
    }
}

However, p_state->stack_size is never reset to 0. Later, when the old GDScriptFunctionState is destroyed, ~GDScriptFunctionState() calls _clear_stack(), which sees stack_size != 0 and destroys the same Variants again — a double-free.

Crash Details

GDScript backtrace:

ERROR: Parameter "_p" is null.
   at: _unref (core/variant/dictionary.cpp:352)
   GDScript backtrace (most recent call first):
       [0] <anonymous lambda> (res://benchmark/common_values.gd:70)
       [1] execute_benchmark (res://benchmark/benchmark.gd:215)

C++ backtrace:

thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  frame #0:  _err_print_error(p_function="_unref", p_file="core/variant/dictionary.cpp", p_line=352, p_error="Parameter \"_p\" is null.")
  frame #1:  _err_print_error
  frame #2:  Dictionary::_unref at dictionary.cpp:352
  frame #3:  Dictionary::~Dictionary at dictionary.cpp:780
  frame #4:  Dictionary::~Dictionary at dictionary.cpp:779
  frame #5:  Variant::_clear_internal at variant.cpp:1433
  frame #6:  Variant::~Variant at variant.h:875
  frame #7:  Variant::~Variant at variant.h:873
  frame #8:  GDScriptFunctionState::_clear_stack at gdscript_function.cpp:366
  frame #9:  GDScriptFunctionState::~GDScriptFunctionState at gdscript_function.cpp:399
  frame #10: GDScriptFunctionState::~GDScriptFunctionState at gdscript_function.cpp:394
  frame #11: memdelete<RefCounted> at memory.h:152
  frame #12: Variant::ObjData::unref at variant.cpp:1123
  frame #13: Variant::_clear_internal at variant.cpp:1418
  frame #14: Variant::~Variant at variant.h:875
  frame #15: Variant::~Variant at variant.h:873
  frame #16: destruct_arr_placement<Variant> at memory.h:242
  frame #17: CowData<Variant>::_unref at cowdata.h:249
  frame #18: CowData<Variant>::~CowData at cowdata.h:216
  frame #19: CowData<Variant>::~CowData at cowdata.h:216
  frame #20: Vector<Variant>::~Vector
  frame #21: Vector<Variant>::~Vector
  frame #22: CallableCustomBind::~CallableCustomBind at callable_bind.h:36
  frame #23: CallableCustomBind::~CallableCustomBind at callable_bind.h:36
  frame #24: memdelete<CallableCustom> at memory.h:152
  frame #25: Callable::~Callable at callable.cpp:443
  frame #26: Callable::~Callable at callable.cpp:440
  frame #27: Object::emit_signalp(p_name="timeout") at object.cpp:1414
  frame #28: Object::emit_signal<>(p_name="timeout") at object.h:977
  frame #29: SceneTree::process_timers at scene_tree.cpp:811

The chain: timer timeout → destroys one-shot slot callable → releases CallableCustomBind holding a Ref<GDScriptFunctionState>~GDScriptFunctionState calls _clear_stack() → attempts to destroy already-freed stack Variants.

Fix

Set p_state->stack_size = 0 after the stack is freed, preventing the redundant destruction in _clear_stack().

Note

🤖 AI disclosure: I used API to assist with troubleshooting and identifying the fix

@stuartcarnie stuartcarnie requested a review from a team as a code owner March 12, 2026 00:53
@dalexeev
Copy link
Copy Markdown
Member

@stuartcarnie
Copy link
Copy Markdown
Contributor Author

awesome, @dalexeev - closing this in favour of #117053

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants