Skip to content

Commit db9b9b6

Browse files
limbonautreduz
andcommitted
GDScript call stack as reverse linked list with fixed coroutines
* GDScript call stack as reverse linked list with issues fixed (originally proposed in 91006). * Fix coroutine issues with call stack by resuming async call chain inside `GDScriptFunction::call()`. * This fixes corrupted line numbers for coroutines in the debugger and backtrace (106489). Co-authored-by: Juan Linietsky <[email protected]>
1 parent 45fc515 commit db9b9b6

File tree

7 files changed

+92
-80
lines changed

7 files changed

+92
-80
lines changed

doc/classes/ProjectSettings.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,6 @@
651651
</member>
652652
<member name="debug/settings/gdscript/always_track_call_stacks" type="bool" setter="" getter="" default="false">
653653
Whether GDScript call stacks will be tracked in release builds, thus allowing [method Engine.capture_script_backtraces] to function.
654-
Enabling this comes at the cost of roughly 40 KiB for every thread that runs any GDScript code.
655654
[b]Note:[/b] This setting has no effect on editor builds or debug builds, where GDScript call stacks are tracked regardless.
656655
</member>
657656
<member name="debug/settings/gdscript/always_track_local_variables" type="bool" setter="" getter="" default="false">

modules/gdscript/gdscript.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,8 +2327,6 @@ void GDScriptLanguage::finish() {
23272327
}
23282328
finishing = true;
23292329

2330-
_call_stack.free();
2331-
23322330
// Clear the cache before parsing the script_list
23332331
GDScriptCache::clear();
23342332

@@ -2921,7 +2919,19 @@ String GDScriptLanguage::get_global_class_name(const String &p_path, String *r_b
29212919
return c->identifier != nullptr ? String(c->identifier->name) : String();
29222920
}
29232921

2924-
thread_local GDScriptLanguage::CallStack GDScriptLanguage::_call_stack;
2922+
thread_local GDScriptLanguage::CallLevel *GDScriptLanguage::_call_stack = nullptr;
2923+
thread_local uint32_t GDScriptLanguage::_call_stack_size = 0;
2924+
2925+
GDScriptLanguage::CallLevel *GDScriptLanguage::_get_stack_level(uint32_t p_level) {
2926+
ERR_FAIL_UNSIGNED_INDEX_V(p_level, _call_stack_size, nullptr);
2927+
CallLevel *level = _call_stack; // Start from top
2928+
uint32_t level_index = 0;
2929+
while (p_level > level_index) {
2930+
level_index++;
2931+
level = level->prev;
2932+
}
2933+
return level;
2934+
}
29252935

29262936
GDScriptLanguage::GDScriptLanguage() {
29272937
ERR_FAIL_COND(singleton);

modules/gdscript/gdscript.h

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -437,31 +437,22 @@ class GDScriptLanguage : public ScriptLanguage {
437437
GDScriptInstance *instance = nullptr;
438438
int *ip = nullptr;
439439
int *line = nullptr;
440+
CallLevel *prev = nullptr; // Reverse linked list (stack).
440441
};
441442

442443
static thread_local int _debug_parse_err_line;
443444
static thread_local String _debug_parse_err_file;
444445
static thread_local String _debug_error;
445-
struct CallStack {
446-
CallLevel *levels = nullptr;
447-
int stack_pos = 0;
448-
449-
void free() {
450-
if (levels) {
451-
memdelete_arr(levels);
452-
levels = nullptr;
453-
}
454-
}
455-
~CallStack() {
456-
free();
457-
}
458-
};
459446

460-
static thread_local CallStack _call_stack;
461-
int _debug_max_call_stack = 0;
447+
static thread_local CallLevel *_call_stack;
448+
static thread_local uint32_t _call_stack_size;
449+
uint32_t _debug_max_call_stack = 0;
450+
462451
bool track_call_stack = false;
463452
bool track_locals = false;
464453

454+
static CallLevel *_get_stack_level(uint32_t p_level);
455+
465456
void _add_global(const StringName &p_name, const Variant &p_value);
466457
void _remove_global(const StringName &p_name);
467458

@@ -492,23 +483,20 @@ class GDScriptLanguage : public ScriptLanguage {
492483
bool debug_break(const String &p_error, bool p_allow_continue = true);
493484
bool debug_break_parse(const String &p_file, int p_line, const String &p_error);
494485

495-
_FORCE_INLINE_ void enter_function(GDScriptInstance *p_instance, GDScriptFunction *p_function, Variant *p_stack, int *p_ip, int *p_line) {
486+
_FORCE_INLINE_ void enter_function(CallLevel *call_level, GDScriptInstance *p_instance, GDScriptFunction *p_function, Variant *p_stack, int *p_ip, int *p_line) {
496487
if (!track_call_stack) {
497488
return;
498489
}
499490

500-
if (unlikely(_call_stack.levels == nullptr)) {
501-
_call_stack.levels = memnew_arr(CallLevel, _debug_max_call_stack + 1);
502-
}
503-
504491
#ifdef DEBUG_ENABLED
505492
ScriptDebugger *script_debugger = EngineDebugger::get_script_debugger();
506493
if (script_debugger != nullptr && script_debugger->get_lines_left() > 0 && script_debugger->get_depth() >= 0) {
507494
script_debugger->set_depth(script_debugger->get_depth() + 1);
508495
}
509496
#endif
510497

511-
if (unlikely(_call_stack.stack_pos >= _debug_max_call_stack)) {
498+
if (unlikely(_call_stack_size >= _debug_max_call_stack)) {
499+
// stack overflow
512500
_debug_error = vformat("Stack overflow (stack size: %s). Check for infinite recursion in your script.", _debug_max_call_stack);
513501

514502
#ifdef DEBUG_ENABLED
@@ -520,13 +508,14 @@ class GDScriptLanguage : public ScriptLanguage {
520508
return;
521509
}
522510

523-
CallLevel &call_level = _call_stack.levels[_call_stack.stack_pos];
524-
call_level.stack = p_stack;
525-
call_level.instance = p_instance;
526-
call_level.function = p_function;
527-
call_level.ip = p_ip;
528-
call_level.line = p_line;
529-
_call_stack.stack_pos++;
511+
call_level->prev = _call_stack;
512+
_call_stack = call_level;
513+
call_level->stack = p_stack;
514+
call_level->instance = p_instance;
515+
call_level->function = p_function;
516+
call_level->ip = p_ip;
517+
call_level->line = p_line;
518+
_call_stack_size++;
530519
}
531520

532521
_FORCE_INLINE_ void exit_function() {
@@ -536,35 +525,42 @@ class GDScriptLanguage : public ScriptLanguage {
536525

537526
#ifdef DEBUG_ENABLED
538527
ScriptDebugger *script_debugger = EngineDebugger::get_script_debugger();
539-
if (script_debugger != nullptr && script_debugger->get_lines_left() > 0 && script_debugger->get_depth() >= 0) {
528+
if (script_debugger && script_debugger->get_lines_left() > 0 && script_debugger->get_depth() >= 0) {
540529
script_debugger->set_depth(script_debugger->get_depth() - 1);
541530
}
542531
#endif
543532

544-
if (unlikely(_call_stack.stack_pos == 0)) {
545-
_debug_error = "Stack Underflow (Engine Bug)";
546-
533+
if (unlikely(_call_stack_size == 0)) {
547534
#ifdef DEBUG_ENABLED
548-
if (script_debugger != nullptr) {
535+
if (script_debugger) {
536+
_debug_error = "Stack Underflow (Engine Bug)";
549537
script_debugger->debug(this);
538+
} else {
539+
ERR_PRINT("Stack underflow! (Engine Bug)");
550540
}
541+
#else // !DEBUG_ENABLED
542+
ERR_PRINT("Stack underflow! (Engine Bug)");
551543
#endif
552-
553544
return;
554545
}
555546

556-
_call_stack.stack_pos--;
547+
_call_stack_size--;
548+
_call_stack = _call_stack->prev;
557549
}
558550

559551
virtual Vector<StackInfo> debug_get_current_stack_info() override {
560552
Vector<StackInfo> csi;
561-
csi.resize(_call_stack.stack_pos);
562-
for (int i = 0; i < _call_stack.stack_pos; i++) {
563-
csi.write[_call_stack.stack_pos - i - 1].line = _call_stack.levels[i].line ? *_call_stack.levels[i].line : 0;
564-
if (_call_stack.levels[i].function) {
565-
csi.write[_call_stack.stack_pos - i - 1].func = _call_stack.levels[i].function->get_name();
566-
csi.write[_call_stack.stack_pos - i - 1].file = _call_stack.levels[i].function->get_script()->get_script_path();
553+
csi.resize(_call_stack_size);
554+
CallLevel *cl = _call_stack;
555+
uint32_t idx = 0;
556+
while (cl) {
557+
csi.write[idx].line = *cl->line;
558+
if (cl->function) {
559+
csi.write[idx].func = cl->function->get_name();
560+
csi.write[idx].file = cl->function->get_script()->get_script_path();
567561
}
562+
idx++;
563+
cl = cl->prev;
568564
}
569565
return csi;
570566
}

modules/gdscript/gdscript_editor.cpp

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -292,57 +292,54 @@ int GDScriptLanguage::debug_get_stack_level_count() const {
292292
return 1;
293293
}
294294

295-
return _call_stack.stack_pos;
295+
return _call_stack_size;
296296
}
297297

298298
int GDScriptLanguage::debug_get_stack_level_line(int p_level) const {
299299
if (_debug_parse_err_line >= 0) {
300300
return _debug_parse_err_line;
301301
}
302302

303-
ERR_FAIL_INDEX_V(p_level, _call_stack.stack_pos, -1);
303+
ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, -1);
304304

305-
int l = _call_stack.stack_pos - p_level - 1;
306-
307-
return *(_call_stack.levels[l].line);
305+
return *(_get_stack_level(p_level)->line);
308306
}
309307

310308
String GDScriptLanguage::debug_get_stack_level_function(int p_level) const {
311309
if (_debug_parse_err_line >= 0) {
312310
return "";
313311
}
314312

315-
ERR_FAIL_INDEX_V(p_level, _call_stack.stack_pos, "");
316-
int l = _call_stack.stack_pos - p_level - 1;
317-
return _call_stack.levels[l].function->get_name();
313+
ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, "");
314+
GDScriptFunction *func = _get_stack_level(p_level)->function;
315+
return func ? func->get_name().operator String() : "";
318316
}
319317

320318
String GDScriptLanguage::debug_get_stack_level_source(int p_level) const {
321319
if (_debug_parse_err_line >= 0) {
322320
return _debug_parse_err_file;
323321
}
324322

325-
ERR_FAIL_INDEX_V(p_level, _call_stack.stack_pos, "");
326-
int l = _call_stack.stack_pos - p_level - 1;
327-
return _call_stack.levels[l].function->get_source();
323+
ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, "");
324+
return _get_stack_level(p_level)->function->get_source();
328325
}
329326

330327
void GDScriptLanguage::debug_get_stack_level_locals(int p_level, List<String> *p_locals, List<Variant> *p_values, int p_max_subitems, int p_max_depth) {
331328
if (_debug_parse_err_line >= 0) {
332329
return;
333330
}
334331

335-
ERR_FAIL_INDEX(p_level, _call_stack.stack_pos);
336-
int l = _call_stack.stack_pos - p_level - 1;
332+
ERR_FAIL_INDEX(p_level, (int)_call_stack_size);
337333

338-
GDScriptFunction *f = _call_stack.levels[l].function;
334+
CallLevel *cl = _get_stack_level(p_level);
335+
GDScriptFunction *f = cl->function;
339336

340337
List<Pair<StringName, int>> locals;
341338

342-
f->debug_get_stack_member_state(*_call_stack.levels[l].line, &locals);
339+
f->debug_get_stack_member_state(*cl->line, &locals);
343340
for (const Pair<StringName, int> &E : locals) {
344341
p_locals->push_back(E.first);
345-
p_values->push_back(_call_stack.levels[l].stack[E.second]);
342+
p_values->push_back(cl->stack[E.second]);
346343
}
347344
}
348345

@@ -351,10 +348,10 @@ void GDScriptLanguage::debug_get_stack_level_members(int p_level, List<String> *
351348
return;
352349
}
353350

354-
ERR_FAIL_INDEX(p_level, _call_stack.stack_pos);
355-
int l = _call_stack.stack_pos - p_level - 1;
351+
ERR_FAIL_INDEX(p_level, (int)_call_stack_size);
356352

357-
GDScriptInstance *instance = _call_stack.levels[l].instance;
353+
CallLevel *cl = _get_stack_level(p_level);
354+
GDScriptInstance *instance = cl->instance;
358355

359356
if (!instance) {
360357
return;
@@ -376,12 +373,9 @@ ScriptInstance *GDScriptLanguage::debug_get_stack_level_instance(int p_level) {
376373
return nullptr;
377374
}
378375

379-
ERR_FAIL_INDEX_V(p_level, _call_stack.stack_pos, nullptr);
380-
381-
int l = _call_stack.stack_pos - p_level - 1;
382-
ScriptInstance *instance = _call_stack.levels[l].instance;
376+
ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, nullptr);
383377

384-
return instance;
378+
return _get_stack_level(p_level)->instance;
385379
}
386380

387381
void GDScriptLanguage::debug_get_globals(List<String> *p_globals, List<Variant> *p_values, int p_max_subitems, int p_max_depth) {

modules/gdscript/gdscript_function.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) {
223223
GDScriptFunctionState *gdfs = Object::cast_to<GDScriptFunctionState>(ret);
224224
if (gdfs && gdfs->function == function) {
225225
completed = false;
226+
// Keep the first state alive via reference.
226227
gdfs->first_state = first_state.is_valid() ? first_state : Ref<GDScriptFunctionState>(this);
227228
}
228229
}
@@ -231,14 +232,6 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) {
231232
state.result = Variant();
232233

233234
if (completed) {
234-
if (first_state.is_valid()) {
235-
first_state->emit_signal(SNAME("completed"), ret);
236-
} else {
237-
emit_signal(SNAME("completed"), ret);
238-
}
239-
240-
GDScriptLanguage::get_singleton()->exit_function();
241-
242235
_clear_stack();
243236
}
244237

modules/gdscript/gdscript_function.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ class GDScriptFunction {
558558
static constexpr int MAX_CALL_DEPTH = 2048; // Limit to try to avoid crash because of a stack overflow.
559559

560560
struct CallState {
561+
Signal completed;
561562
GDScript *script = nullptr;
562563
GDScriptInstance *instance = nullptr;
563564
#ifdef DEBUG_ENABLED

modules/gdscript/gdscript_vm.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,8 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
621621

622622
String err_text;
623623

624-
GDScriptLanguage::get_singleton()->enter_function(p_instance, this, stack, &ip, &line);
624+
GDScriptLanguage::CallLevel call_level;
625+
GDScriptLanguage::get_singleton()->enter_function(&call_level, p_instance, this, stack, &ip, &line);
625626

626627
#ifdef DEBUG_ENABLED
627628
#define GD_ERR_BREAK(m_cond) \
@@ -2508,7 +2509,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
25082509
// Is this even possible to be null at this point?
25092510
if (obj) {
25102511
if (obj->is_class_ptr(GDScriptFunctionState::get_class_ptr_static())) {
2511-
result = Signal(obj, "completed");
2512+
result = Signal(obj, SNAME("completed"));
25122513
}
25132514
}
25142515
}
@@ -2555,6 +2556,13 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
25552556
gdfs->state.defarg = defarg;
25562557
gdfs->function = this;
25572558

2559+
if (p_state) {
2560+
// Pass down the signal from the first state.
2561+
gdfs->state.completed = p_state->completed;
2562+
} else {
2563+
gdfs->state.completed = Signal(gdfs.ptr(), SNAME("completed"));
2564+
}
2565+
25582566
retvalue = gdfs;
25592567

25602568
Error err = sig.connect(Callable(gdfs.ptr(), "_signal_callback").bind(retvalue), Object::CONNECT_ONE_SHOT);
@@ -3883,5 +3891,16 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
38833891

38843892
call_depth--;
38853893

3894+
if (p_state && !awaited) {
3895+
// This means we have finished executing a resumed function and it was not awaited again.
3896+
3897+
// Signal the next function-state to resume.
3898+
const Variant *args[1] = { &retvalue };
3899+
p_state->completed.emit(args, 1);
3900+
3901+
// Exit function only after executing the remaining function states to preserve async call stack.
3902+
GDScriptLanguage::get_singleton()->exit_function();
3903+
}
3904+
38863905
return retvalue;
38873906
}

0 commit comments

Comments
 (0)