Skip to content

Commit bfe66ab

Browse files
RandomShaperRandomShaper
committed
Fixup thread-owned lambda bookkeeping on thread exit (take 2)
1 parent f26328e commit bfe66ab

File tree

3 files changed

+129
-17
lines changed

3 files changed

+129
-17
lines changed

‎core/templates/list.h‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@ class List {
132132
data->erase(this);
133133
}
134134

135+
void transfer_to_back(List<T, A> *p_dst_list);
136+
135137
_FORCE_INLINE_ Element() {}
136138
};
137139

@@ -762,4 +764,41 @@ class List {
762764
}
763765
};
764766

767+
template <class T, class A>
768+
void List<T, A>::Element::transfer_to_back(List<T, A> *p_dst_list) {
769+
// Detach from current.
770+
771+
if (data->first == this) {
772+
data->first = data->first->next_ptr;
773+
}
774+
if (data->last == this) {
775+
data->last = data->last->prev_ptr;
776+
}
777+
if (prev_ptr) {
778+
prev_ptr->next_ptr = next_ptr;
779+
}
780+
if (next_ptr) {
781+
next_ptr->prev_ptr = prev_ptr;
782+
}
783+
data->size_cache--;
784+
785+
// Attach to the back of the new one.
786+
787+
if (!p_dst_list->_data) {
788+
p_dst_list->_data = memnew_allocator(_Data, A);
789+
p_dst_list->_data->first = this;
790+
p_dst_list->_data->last = nullptr;
791+
p_dst_list->_data->size_cache = 0;
792+
prev_ptr = nullptr;
793+
} else {
794+
p_dst_list->_data->last->next_ptr = this;
795+
prev_ptr = p_dst_list->_data->last;
796+
}
797+
p_dst_list->_data->last = this;
798+
next_ptr = nullptr;
799+
800+
data = p_dst_list->_data;
801+
p_dst_list->_data->size_cache++;
802+
}
803+
765804
#endif // LIST_H

‎modules/gdscript/gdscript.cpp‎

Lines changed: 74 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,36 +1391,54 @@ String GDScript::debug_get_script_name(const Ref<Script> &p_script) {
13911391
}
13921392
#endif
13931393

1394-
thread_local GDScript::UpdatableFuncPtr GDScript::func_ptrs_to_update_thread_local;
1394+
GDScript::UpdatableFuncPtr GDScript::func_ptrs_to_update_main_thread;
1395+
thread_local GDScript::UpdatableFuncPtr *GDScript::func_ptrs_to_update_thread_local = nullptr;
13951396

13961397
GDScript::UpdatableFuncPtrElement GDScript::_add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr) {
13971398
UpdatableFuncPtrElement result = {};
13981399

13991400
{
1400-
MutexLock lock(func_ptrs_to_update_thread_local.mutex);
1401-
result.element = func_ptrs_to_update_thread_local.ptrs.push_back(p_func_ptr_ptr);
1402-
result.mutex = &func_ptrs_to_update_thread_local.mutex;
1401+
MutexLock lock(func_ptrs_to_update_thread_local->mutex);
1402+
result.element = func_ptrs_to_update_thread_local->ptrs.push_back(p_func_ptr_ptr);
1403+
result.func_ptr = func_ptrs_to_update_thread_local;
14031404

1404-
if (likely(func_ptrs_to_update_thread_local.initialized)) {
1405+
if (likely(func_ptrs_to_update_thread_local->initialized)) {
14051406
return result;
14061407
}
14071408

1408-
func_ptrs_to_update_thread_local.initialized = true;
1409+
func_ptrs_to_update_thread_local->initialized = true;
14091410
}
14101411

14111412
MutexLock lock(func_ptrs_to_update_mutex);
1412-
func_ptrs_to_update.push_back(&func_ptrs_to_update_thread_local);
1413+
func_ptrs_to_update.push_back(func_ptrs_to_update_thread_local);
1414+
func_ptrs_to_update_thread_local->rc++;
14131415

14141416
return result;
14151417
}
14161418

1417-
void GDScript::_remove_func_ptr_to_update(const UpdatableFuncPtrElement p_func_ptr_element) {
1419+
void GDScript::_remove_func_ptr_to_update(const UpdatableFuncPtrElement &p_func_ptr_element) {
14181420
ERR_FAIL_NULL(p_func_ptr_element.element);
1419-
ERR_FAIL_NULL(p_func_ptr_element.mutex);
1420-
MutexLock lock(*p_func_ptr_element.mutex);
1421+
ERR_FAIL_NULL(p_func_ptr_element.func_ptr);
1422+
MutexLock lock(p_func_ptr_element.func_ptr->mutex);
14211423
p_func_ptr_element.element->erase();
14221424
}
14231425

1426+
void GDScript::_fixup_thread_function_bookkeeping() {
1427+
// Transfer the ownership of these update items to the main thread,
1428+
// because the current one is dying, leaving theirs orphan, dangling.
1429+
1430+
DEV_ASSERT(!Thread::is_main_thread());
1431+
1432+
MutexLock lock(func_ptrs_to_update_main_thread.mutex);
1433+
MutexLock lock2(func_ptrs_to_update_thread_local->mutex);
1434+
1435+
while (!func_ptrs_to_update_thread_local->ptrs.is_empty()) {
1436+
List<GDScriptFunction **>::Element *E = func_ptrs_to_update_thread_local->ptrs.front();
1437+
E->transfer_to_back(&func_ptrs_to_update_main_thread.ptrs);
1438+
func_ptrs_to_update_thread_local->transferred = true;
1439+
}
1440+
}
1441+
14241442
void GDScript::clear(ClearData *p_clear_data) {
14251443
if (clearing) {
14261444
return;
@@ -1441,9 +1459,27 @@ void GDScript::clear(ClearData *p_clear_data) {
14411459
{
14421460
MutexLock outer_lock(func_ptrs_to_update_mutex);
14431461
for (UpdatableFuncPtr *updatable : func_ptrs_to_update) {
1444-
MutexLock inner_lock(updatable->mutex);
1445-
for (GDScriptFunction **func_ptr_ptr : updatable->ptrs) {
1446-
*func_ptr_ptr = nullptr;
1462+
bool destroy = false;
1463+
{
1464+
MutexLock inner_lock(updatable->mutex);
1465+
if (updatable->transferred) {
1466+
func_ptrs_to_update_main_thread.mutex.lock();
1467+
}
1468+
for (GDScriptFunction **func_ptr_ptr : updatable->ptrs) {
1469+
*func_ptr_ptr = nullptr;
1470+
}
1471+
DEV_ASSERT(updatable->rc != 0);
1472+
updatable->rc--;
1473+
if (updatable->rc == 0) {
1474+
destroy = true;
1475+
}
1476+
if (updatable->transferred) {
1477+
func_ptrs_to_update_main_thread.mutex.unlock();
1478+
}
1479+
}
1480+
if (destroy) {
1481+
DEV_ASSERT(updatable != &func_ptrs_to_update_main_thread);
1482+
memdelete(updatable);
14471483
}
14481484
}
14491485
}
@@ -2065,6 +2101,27 @@ void GDScriptLanguage::remove_named_global_constant(const StringName &p_name) {
20652101
named_globals.erase(p_name);
20662102
}
20672103

2104+
void GDScriptLanguage::thread_enter() {
2105+
GDScript::func_ptrs_to_update_thread_local = memnew(GDScript::UpdatableFuncPtr);
2106+
}
2107+
2108+
void GDScriptLanguage::thread_exit() {
2109+
GDScript::_fixup_thread_function_bookkeeping();
2110+
2111+
bool destroy = false;
2112+
{
2113+
MutexLock lock(GDScript::func_ptrs_to_update_thread_local->mutex);
2114+
DEV_ASSERT(GDScript::func_ptrs_to_update_thread_local->rc != 0);
2115+
GDScript::func_ptrs_to_update_thread_local->rc--;
2116+
if (GDScript::func_ptrs_to_update_thread_local->rc == 0) {
2117+
destroy = true;
2118+
}
2119+
}
2120+
if (destroy) {
2121+
memdelete(GDScript::func_ptrs_to_update_thread_local);
2122+
}
2123+
}
2124+
20682125
void GDScriptLanguage::init() {
20692126
//populate global constants
20702127
int gcc = CoreConstants::get_global_constant_count();
@@ -2097,6 +2154,8 @@ void GDScriptLanguage::init() {
20972154
_add_global(E.name, E.ptr);
20982155
}
20992156

2157+
GDScript::func_ptrs_to_update_thread_local = &GDScript::func_ptrs_to_update_main_thread;
2158+
21002159
#ifdef TESTS_ENABLED
21012160
GDScriptTests::GDScriptTestRunner::handle_cmdline();
21022161
#endif
@@ -2146,6 +2205,8 @@ void GDScriptLanguage::finish() {
21462205
}
21472206
script_list.clear();
21482207
function_list.clear();
2208+
2209+
DEV_ASSERT(GDScript::func_ptrs_to_update_main_thread.rc == 1);
21492210
}
21502211

21512212
void GDScriptLanguage::profiling_start() {

‎modules/gdscript/gdscript.h‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,18 +121,25 @@ class GDScript : public Script {
121121
struct UpdatableFuncPtr {
122122
List<GDScriptFunction **> ptrs;
123123
Mutex mutex;
124-
bool initialized = false;
124+
bool initialized : 1;
125+
bool transferred : 1;
126+
uint32_t rc = 1;
127+
UpdatableFuncPtr() :
128+
initialized(false), transferred(false) {}
125129
};
126130
struct UpdatableFuncPtrElement {
127131
List<GDScriptFunction **>::Element *element = nullptr;
128-
Mutex *mutex = nullptr;
132+
UpdatableFuncPtr *func_ptr = nullptr;
129133
};
130-
static thread_local UpdatableFuncPtr func_ptrs_to_update_thread_local;
134+
static UpdatableFuncPtr func_ptrs_to_update_main_thread;
135+
static thread_local UpdatableFuncPtr *func_ptrs_to_update_thread_local;
131136
List<UpdatableFuncPtr *> func_ptrs_to_update;
132137
Mutex func_ptrs_to_update_mutex;
133138

134139
UpdatableFuncPtrElement _add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr);
135-
static void _remove_func_ptr_to_update(const UpdatableFuncPtrElement p_func_ptr_element);
140+
static void _remove_func_ptr_to_update(const UpdatableFuncPtrElement &p_func_ptr_element);
141+
142+
static void _fixup_thread_function_bookkeeping();
136143

137144
#ifdef TOOLS_ENABLED
138145
// For static data storage during hot-reloading.
@@ -554,6 +561,11 @@ class GDScriptLanguage : public ScriptLanguage {
554561
virtual void add_named_global_constant(const StringName &p_name, const Variant &p_value) override;
555562
virtual void remove_named_global_constant(const StringName &p_name) override;
556563

564+
/* MULTITHREAD FUNCTIONS */
565+
566+
virtual void thread_enter() override;
567+
virtual void thread_exit() override;
568+
557569
/* DEBUGGER FUNCTIONS */
558570

559571
virtual String debug_get_error() const override;

0 commit comments

Comments
 (0)