Skip to content

Commit 93166a8

Browse files
committed
Add ability to suspend threads until a signal is called
For GDScript users that run code on threads, it often happens they want to `await` until something happens, the continue the thread execution. Unfortunately, signals will most likely execute on the main thread, hence this means that the code being run on the thread will continue running on the main thread by default. It has been discussed whether await should be smart about this and simply suspend the thread if running on one. The problem with this is, that users may also be willing to emit the signal that will resume from the thread itself and, at the time of awaiting, there is **no way for the interpreter to know on which thread the function will be resumed**. Additionally, suspending the thread is a very different operation than awaiting. Awaiting saves the local function stack and returns immediately, while suspending stops the whole thread until another resumes it. Mixing and matching both seems, ultimately, undesired as they are not the same. To solve this, a new utility function is added by this pull request, which suspends a thread until a signal is emitted (no matter in which other thread). It works like this. ```GDScript # Suspends a thread until a button is pressed. Thread.suspend( button.pressed ) ``` If code must do this and can run on both the main thread or a dedicated thread, it can do as follows: ```GDScript # Suspends a thread until a button is pressed. if (Thread.is_main_thread()): await button.pressed else: Thread.suspend( button.pressed ) ``` For this, the `Thread.is_main_thread()` function (already existing in the internal Thread object) has been exposed to the engine API. **Q**: Are you sure this can't be done with await transparently? **A**: No, please read again. There is no way for await to know beforehand how the function will be resumed. Only the user knows. **Q**: Does it not make more sense to have an `await_thread` or something where the user will pass the argument? **A**: I think its better to have a dedicated utility function for this. Not only the intention is more explicit, but additionally, as this makes it non exclusive to GDScript. It can be used from other languages that use the engine.
1 parent cae3d72 commit 93166a8

8 files changed

+130
-0
lines changed

core/core_bind.cpp

+91
Original file line numberDiff line numberDiff line change
@@ -1391,6 +1391,10 @@ bool Thread::is_alive() const {
13911391
return running.is_set();
13921392
}
13931393

1394+
bool Thread::is_main_thread() {
1395+
return ::Thread::is_main_thread();
1396+
}
1397+
13941398
Variant Thread::wait_to_finish() {
13951399
ERR_FAIL_COND_V_MSG(!is_started(), Variant(), "Thread must have been started to wait for its completion.");
13961400
thread.wait_to_finish();
@@ -1405,6 +1409,91 @@ void Thread::set_thread_safety_checks_enabled(bool p_enabled) {
14051409
set_current_thread_safe_for_nodes(!p_enabled);
14061410
}
14071411

1412+
class CallableCustomSuspend : public CallableCustom {
1413+
Semaphore *semaphore = nullptr;
1414+
Variant *return_value = nullptr;
1415+
1416+
// Never really going to execute since disconnection is automatic.
1417+
static bool _equal_func(const CallableCustom *p_a, const CallableCustom *p_b) {
1418+
const CallableCustomSuspend *A = static_cast<const CallableCustomSuspend *>(p_a);
1419+
const CallableCustomSuspend *B = static_cast<const CallableCustomSuspend *>(p_b);
1420+
1421+
return A->semaphore == B->semaphore;
1422+
}
1423+
1424+
// Never really going to execute since disconnection is automatic.
1425+
static bool _less_func(const CallableCustom *p_a, const CallableCustom *p_b) {
1426+
const CallableCustomSuspend *A = static_cast<const CallableCustomSuspend *>(p_a);
1427+
const CallableCustomSuspend *B = static_cast<const CallableCustomSuspend *>(p_b);
1428+
1429+
return A->semaphore < B->semaphore;
1430+
}
1431+
1432+
public:
1433+
//for every type that inherits, these must always be the same for this type
1434+
virtual uint32_t hash() const override {
1435+
return size_t(semaphore);
1436+
}
1437+
1438+
virtual String get_as_text() const override {
1439+
return "SemaphoreCallable";
1440+
}
1441+
1442+
virtual CompareEqualFunc get_compare_equal_func() const override {
1443+
return _equal_func;
1444+
}
1445+
1446+
virtual CompareLessFunc get_compare_less_func() const override {
1447+
return _less_func;
1448+
}
1449+
1450+
virtual ObjectID get_object() const override {
1451+
return ObjectID();
1452+
}
1453+
1454+
virtual bool is_valid() const override {
1455+
return true;
1456+
}
1457+
1458+
virtual void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, Callable::CallError &r_call_error) const override {
1459+
semaphore->post();
1460+
if (p_argcount == 1) { // If passed one argument, will be returned.
1461+
if (return_value) {
1462+
*return_value = *p_arguments[0];
1463+
}
1464+
} else if (p_argcount > 1) {
1465+
r_call_error.error = Callable::CallError::CALL_ERROR_TOO_MANY_ARGUMENTS;
1466+
r_call_error.argument = p_argcount;
1467+
r_call_error.expected = 1;
1468+
return;
1469+
}
1470+
1471+
r_call_error.error = Callable::CallError::CALL_OK;
1472+
}
1473+
1474+
CallableCustomSuspend(Semaphore *p_semaphore, Variant *r_return_value) {
1475+
semaphore = p_semaphore;
1476+
return_value = r_return_value;
1477+
}
1478+
};
1479+
1480+
Variant Thread::suspend(Signal p_resume_signal, bool p_connect_on_main_thread) {
1481+
Semaphore semaphore;
1482+
Variant return_value;
1483+
1484+
Callable callable(memnew(CallableCustomSuspend(&semaphore, &return_value)));
1485+
if (p_connect_on_main_thread) {
1486+
ObjectID obj_id = p_resume_signal.get_object_id();
1487+
MessageQueue::get_singleton()->push_call(obj_id, SNAME("connect"), p_resume_signal.get_name(), callable, CONNECT_ONE_SHOT);
1488+
} else {
1489+
p_resume_signal.connect(callable, Object::CONNECT_ONE_SHOT);
1490+
}
1491+
1492+
semaphore.wait();
1493+
1494+
return return_value;
1495+
}
1496+
14081497
void Thread::_bind_methods() {
14091498
ClassDB::bind_method(D_METHOD("start", "callable", "priority"), &Thread::start, DEFVAL(PRIORITY_NORMAL));
14101499
ClassDB::bind_method(D_METHOD("get_id"), &Thread::get_id);
@@ -1413,6 +1502,8 @@ void Thread::_bind_methods() {
14131502
ClassDB::bind_method(D_METHOD("wait_to_finish"), &Thread::wait_to_finish);
14141503

14151504
ClassDB::bind_static_method("Thread", D_METHOD("set_thread_safety_checks_enabled", "enabled"), &Thread::set_thread_safety_checks_enabled);
1505+
ClassDB::bind_static_method("Thread", D_METHOD("is_main_thread"), &Thread::is_main_thread);
1506+
ClassDB::bind_static_method("Thread", D_METHOD("suspend", "resume_signal", "connect_on_main_thread"), &Thread::suspend, DEFVAL(true));
14161507

14171508
BIND_ENUM_CONSTANT(PRIORITY_LOW);
14181509
BIND_ENUM_CONSTANT(PRIORITY_NORMAL);

core/core_bind.h

+2
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,8 @@ class Thread : public RefCounted {
453453
bool is_started() const;
454454
bool is_alive() const;
455455
Variant wait_to_finish();
456+
static bool is_main_thread();
457+
static Variant suspend(Signal p_resume_signal, bool p_connect_on_main_thread = true);
456458

457459
static void set_thread_safety_checks_enabled(bool p_enabled);
458460
};

doc/classes/ProjectSettings.xml

+2
Original file line numberDiff line numberDiff line change
@@ -3211,6 +3211,8 @@
32113211
- 8×8 = rgb(255, 255, 0) - #ffff00 - Not supported on most hardware
32123212
[/codeblock]
32133213
</member>
3214+
<member name="threading/warnings/warn_on_cross_thread_await" type="bool" setter="" getter="" default="true">
3215+
</member>
32143216
<member name="threading/worker_pool/low_priority_thread_ratio" type="float" setter="" getter="" default="0.3">
32153217
The ratio of [WorkerThreadPool]'s threads that will be reserved for low-priority tasks. For example, if 10 threads are available and this value is set to [code]0.3[/code], 3 of the worker threads will be reserved for low-priority tasks. The actual value won't exceed the number of CPU cores minus one, and if possible, at least one worker thread will be dedicated to low-priority tasks.
32163218
</member>

doc/classes/Thread.xml

+14
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@
3030
To check if a [Thread] is joinable, use [method is_started].
3131
</description>
3232
</method>
33+
<method name="is_main_thread" qualifiers="static">
34+
<return type="bool" />
35+
<description>
36+
Returns [code]true[/code] if the function is called from the main thread of the engine. This is the thread that runs game code by default.
37+
</description>
38+
</method>
3339
<method name="is_started" qualifiers="const">
3440
<return type="bool" />
3541
<description>
@@ -60,6 +66,14 @@
6066
Returns [constant OK] on success, or [constant ERR_CANT_CREATE] on failure.
6167
</description>
6268
</method>
69+
<method name="suspend" qualifiers="static">
70+
<return type="Variant" />
71+
<param index="0" name="resume_signal" type="Signal" />
72+
<param index="1" name="connect_on_main_thread" type="bool" default="true" />
73+
<description>
74+
Suspend a thread until the signal provided in the [param resume_signal] argument is emitted. As connecting signals between threads is often discouraged (or blocked by thread guards), the [param connect_on_main_thread] is provided to perform the connection to the signal on the main thread. As a note, if you intend to await on a script function instead of a signal, use the following syntax instead: [code] Thread.suspend( function().completed ) [/code].
75+
</description>
76+
</method>
6377
<method name="wait_to_finish">
6478
<return type="Variant" />
6579
<description>

modules/gdscript/gdscript_function.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,18 @@ bool GDScriptFunctionState::is_valid(bool p_extended_check) const {
187187
return true;
188188
}
189189

190+
bool GDScriptFunctionState::warn_on_cross_thread_await = true;
191+
192+
void GDScriptFunctionState::set_warn_on_cross_thread_await(bool p_enable) {
193+
warn_on_cross_thread_await = p_enable;
194+
}
195+
190196
Variant GDScriptFunctionState::resume(const Variant &p_arg) {
197+
#ifdef DEBUG_ENABLED
198+
if (warn_on_cross_thread_await && await_id != Thread::get_caller_id()) {
199+
WARN_PRINT("Function " + function->get_name() + " was resumed on a different thread than it was awaited. If you really intend to do this, Use Thread.suspend() for cross thread suspension or disable the 'warn_on_cross_thread_await' warning on project settings.");
200+
}
201+
#endif
191202
ERR_FAIL_NULL_V(function, Variant());
192203
{
193204
MutexLock lock(GDScriptLanguage::singleton->mutex);

modules/gdscript/gdscript_function.h

+5
Original file line numberDiff line numberDiff line change
@@ -608,10 +608,15 @@ class GDScriptFunctionState : public RefCounted {
608608
SelfList<GDScriptFunctionState> scripts_list;
609609
SelfList<GDScriptFunctionState> instances_list;
610610

611+
static bool warn_on_cross_thread_await;
612+
Thread::ID await_id = Thread::UNASSIGNED_ID;
613+
611614
protected:
612615
static void _bind_methods();
613616

614617
public:
618+
static void set_warn_on_cross_thread_await(bool p_enable);
619+
615620
bool is_valid(bool p_extended_check = false) const;
616621
Variant resume(const Variant &p_arg = Variant());
617622

modules/gdscript/gdscript_vm.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -2542,6 +2542,8 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
25422542
gdfs->state.ip = ip + 2;
25432543
gdfs->state.line = line;
25442544
gdfs->state.script = _script;
2545+
gdfs->await_id = Thread::get_caller_id();
2546+
25452547
{
25462548
MutexLock lock(GDScriptLanguage::get_singleton()->mutex);
25472549
_script->pending_func_states.add(&gdfs->scripts_list);

modules/gdscript/register_types.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include "tests/test_gdscript.h"
5050
#endif
5151

52+
#include "core/config/project_settings.h"
5253
#include "core/io/file_access.h"
5354
#include "core/io/resource_loader.h"
5455

@@ -153,6 +154,8 @@ void initialize_gdscript_module(ModuleInitializationLevel p_level) {
153154
gdscript_cache = memnew(GDScriptCache);
154155

155156
GDScriptUtilityFunctions::register_functions();
157+
158+
GDScriptFunctionState::set_warn_on_cross_thread_await(GLOBAL_DEF("threading/warnings/warn_on_cross_thread_await", true));
156159
}
157160

158161
#ifdef TOOLS_ENABLED

0 commit comments

Comments
 (0)