godot: Crash when connecting or disconnecting a signal from multiple threads

This issue is rather problematic. This basically means that I cannot use a Thread to instance nodes that will eventually have to get freed again as there’s always the chance of this signal crash when I free nodes that were instantiated from a thread. And instancing all my nodes from the main thread is not an option as it causes stuttering in my project.

I will continue to try and find workarounds, are there any already known workarounds for this crash? A way to free nodes that were instantiated from a separate thread instance without it potentially crashing the game?

The issue i was having was not created by queue_free in a thread. the signal map becomes corrupted when you call .instance() from the thread because the thread manipulates the signal map without locking it. It shows up later when you free the instance but the actual corruption happens when you create the nodes from a separate thread (in my case)

Some issues with this situation that aren’t easily manageable by explicit thread safe methods, or at all:

• Resources being retrieved from cache, this isn’t a case of it being impossible, but rather one that is hard to keep track of #60098
• Several types (for example AudioStreamPlayer) connect signals to servers unconditionally, on creation #77518
• It also seems to occur in the editor #69614

Simple actions such as attaching a resource to a node, while doing the correct thing of preparing it on a separate thread, not realizing the resource was cached and is not unique, and simply setting the mesh of a MeshInstance can cause this (see #60098), and I don’t necessarily think it’s reasonable to entirely forgo using cached resources for this, nor really to make convoluted code to replace the resource in the main thread

Essentially, there are a bunch of cases where signals are connected, unbeknownst to the user, and trying to navigate the thread safety of that is hard even for experienced users, and the behaviour in this case isn’t something covered by the stability or compatibility of the engine, so a function can suddenly start connecting a signal that it didn’t in the last patch release, throwing everything off

To me this level of unpredictability really warrants making signal connections thread safe, while I do get that explicitly connecting signals should be something the user needs to be aware of isn’t guaranteed to be thread safe, this isn’t necessarily limited to that

A possible (at least temporary) fix would be to provide a function to make the connection in a deferred way, possibly making this thread-aware and connecting directly if on the main thread (I’m realising with this that a general option to do deferred calls only if not on the main thread as a special function might be an interesting idea)

This looks promising: https://www.stroustrup.com/lock-free-vector.pdf Or this: https://github.com/cameron314/concurrentqueue https://www.1024cores.net/home/lock-free-algorithms/queues

Cas operations are slower than no thread safety… but how much slower when contention is rare?

Alternative ideas:

1. to ensure that instantiation of every type of scene node is very fast and cheap. If there were no costly instance() calls (like when you create a viewport node) then the thread safety of the signal connections would not be a problem in this case.
2. defer signal connection when scenes are being instanced and ensure it’s called in the main thread. Signal connection should be fast even if the nodes being instantiated are slow. (Might be fine if you definitely add the new instanced tree fragment to the scene tree in the main thread and the signals have definitely been connected by then? Maybe a lock free queue of signal add/delete operations? If the queue is non empty it can be merged into the main connection list if the main thread is reading/writing. If the non main thread reads or writes then the contents would be considered like a journal?.. getting complicated) A journal could probably be very fast with a single consumer queue: https://www.1024cores.net/home/lock-free-algorithms/queues/intrusive-mpsc-node-based-queue

https://www.1024cores.net/home/lock-free-algorithms/queues/intrusive-mpsc-node-based-queue

These are all very complicated and usually focused more on hpc and high degrees of parallelism… maybe something could be done with a fast path/slow path where the slow path syncs access to a journal with a mutex and the fast path (when the journal is empty) only needs a memory barrier… the journal can be replayed on thread access and committed on the main thread access…

Not a lot of things in Godot are thread-safe (https://docs.godotengine.org/en/stable/tutorials/threads/thread_safe_apis.html). Object is not designed to be. among them, for performance reasons, since that would mean Object’s code would be full of mutexes when the most relevant use case is single threaded access.

In order to manipulate most types of Objects from multiple threads, a project needs to use its own locking mechanism (based on Mutex, Sempahore, etc.). The case of connecting signals from arbitrary threads is a tricky one, since the engine will check the connection map at some point without respecting any sync mechanism the project is using.

So, for anything shared with the engine in such a way that it may access it at hard to predict moments (like signal connections, nodes already in the scene tree, etc.), the right way to operate on it in a multiple threads context (if you really need to) is to create some sort of communicaiton channel between your arbitrary thread and the main one, and to let the latter do what is necessary.

This is a way to approach the example from the OP safely:

extends Node

signal dummy_signal()

# This is for avoiding thrashing the message queue if the threads are faster
# creating objects than the main thread deleting them (in an extreme example
# like this).
# (This could be a singleton with a proper _process() function.)
class DeferredDeleter:
	var objects = []
	var mutex = Mutex.new()

	func add(object):
		mutex.lock()
		objects.append(object)
		mutex.unlock()

	func process():
		mutex.lock()
		for o in objects:
			o.free()
		objects.clear()
		mutex.unlock()

class DummyObject extends Object:
	var sem = Semaphore.new()

	func dummy_method():
		pass

	func do_the_connection(main):
		main.connect("dummy_signal", self, "dummy_method") # dummy method
		print("Connected!")
		sem.post()

var dd = DeferredDeleter.new()

func _ready():
	var threads = []
	for i in range(2):
		var thread = Thread.new()
		thread.start(self, "_thread", null)
		threads.push_back(thread)

func _process(delta):
	print("Working")
	dd.process()

func _thread(_x):
	while true:
		var object = DummyObject.new()
		object.call_deferred("do_the_connection", self)
		object.sem.wait()
		dd.add(object)

UPDATE: Now I’m realizing that the deferred call to do_the_connection() can potentially thrash the engine’s message queue, too. However, I don’t have more time to spend on this example and I guess it already illustrates my point. In any case, if threads are not as aggressive as in this example, that wouldn’t be necessary; and the deferred deleter probably either (call_deferred('free') would be just fine).

That’s pretty much @RandomShaper’s de facto area of expertise now 😃

@Flarkk Unfortunately not! I had this solution where I was constantly instancing scenes from a thread and queue freeing those scenes from the main thread again. Hundreds of times per second. In this case, this issue would occur but rarely. Now I am no longer freeing scenes and I am instead reusing them so I no longer run into this potential crash anymore.

Short messages like malloc(): invalid next->prev_inuse (unsorted) or realloc(): invalid next size means that memory is too damaged to continue the program and the program can crash in random moments(each time may be different backtrace) I found that using Address Sanitizer scons p=x11 -j6 use_asan=yes show a lot of more detailed backtrace of first use invalid memory, double free etc. instead show backtrace of crash, which can happens a lot of time later. This is output from Asan(This time is very similar to crash backtraces)

==4043==ERROR: AddressSanitizer: attempting double-free on 0x61900012fc80 in thread T6 (_thread):
    #0 0x7fc65c28bf1e in __interceptor_realloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10df1e)
    #1 0xea60b75 in Memory::realloc_static(void*, unsigned long, bool) core/os/memory.cpp:137
    #2 0xe581a4e in CowData<VMap<Object::Signal::Target, Object::Signal::Slot>::Pair>::resize(int) core/cowdata.h:283
    #3 0xe5812cb in CowData<VMap<Object::Signal::Target, Object::Signal::Slot>::Pair>::insert(int, VMap<Object::Signal::Target, Object::Signal::Slot>::Pair const&) core/cowdata.h:175
    #4 0xe57233a in VMap<Object::Signal::Target, Object::Signal::Slot>::insert(Object::Signal::Target const&, Object::Signal::Slot const&) core/vmap.h:125
    #5 0xe5688d5 in VMap<Object::Signal::Target, Object::Signal::Slot>::operator[](Object::Signal::Target const&) core/vmap.h:199
    #6 0xe549b1a in Object::connect(StringName const&, Object*, StringName const&, Vector<Variant> const&, unsigned int) core/object.cpp:1485
    #7 0xe595430 in MethodBind5R<Error, StringName const&, Object*, StringName const&, Vector<Variant> const&, unsigned int>::call(Object*, Variant const**, int, Variant::CallError&) core/method_bind.gen.inc:4177
    #8 0xe5373aa in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:921
    #9 0xe7a9e6e in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) core/variant_call.cpp:1112
    #10 0x1a1836a in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_function.cpp:1081
    #11 0x18549c3 in GDScriptInstance::call(StringName const&, Variant const**, int, Variant::CallError&) modules/gdscript/gdscript.cpp:1173
    #12 0xe536f17 in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:900
    #13 0xf057b55 in _Thread::_start_func(void*) core/bind/core_bind.cpp:2664
    #14 0x4f836b5 in ThreadPosix::thread_callback(void*) drivers/unix/thread_posix.cpp:74
    #15 0x7fc65bc18668 in start_thread /build/glibc-4WA41p/glibc-2.30/nptl/pthread_create.c:479
    #16 0x7fc65ae79322 in clone (/lib/x86_64-linux-gnu/libc.so.6+0x122322)

0x61900012fc80 is located 0 bytes inside of 1040-byte region [0x61900012fc80,0x619000130090)
freed by thread T7 (_thread) here:
    #0 0x7fc65c28bf1e in __interceptor_realloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10df1e)
    #1 0xea60b75 in Memory::realloc_static(void*, unsigned long, bool) core/os/memory.cpp:137
    #2 0xe581a4e in CowData<VMap<Object::Signal::Target, Object::Signal::Slot>::Pair>::resize(int) core/cowdata.h:283
    #3 0xe5812cb in CowData<VMap<Object::Signal::Target, Object::Signal::Slot>::Pair>::insert(int, VMap<Object::Signal::Target, Object::Signal::Slot>::Pair const&) core/cowdata.h:175
    #4 0xe57233a in VMap<Object::Signal::Target, Object::Signal::Slot>::insert(Object::Signal::Target const&, Object::Signal::Slot const&) core/vmap.h:125
    #5 0xe5688d5 in VMap<Object::Signal::Target, Object::Signal::Slot>::operator[](Object::Signal::Target const&) core/vmap.h:199
    #6 0xe549b1a in Object::connect(StringName const&, Object*, StringName const&, Vector<Variant> const&, unsigned int) core/object.cpp:1485
    #7 0xe595430 in MethodBind5R<Error, StringName const&, Object*, StringName const&, Vector<Variant> const&, unsigned int>::call(Object*, Variant const**, int, Variant::CallError&) core/method_bind.gen.inc:4177
    #8 0xe5373aa in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:921
    #9 0xe7a9e6e in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) core/variant_call.cpp:1112
    #10 0x1a1836a in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_function.cpp:1081
    #11 0x18549c3 in GDScriptInstance::call(StringName const&, Variant const**, int, Variant::CallError&) modules/gdscript/gdscript.cpp:1173
    #12 0xe536f17 in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:900
    #13 0xf057b55 in _Thread::_start_func(void*) core/bind/core_bind.cpp:2664
    #14 0x4f836b5 in ThreadPosix::thread_callback(void*) drivers/unix/thread_posix.cpp:74
    #15 0x7fc65bc18668 in start_thread /build/glibc-4WA41p/glibc-2.30/nptl/pthread_create.c:479

previously allocated by thread T7 (_thread) here:
    #0 0x7fc65c28bf1e in __interceptor_realloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10df1e)
    #1 0xea60b75 in Memory::realloc_static(void*, unsigned long, bool) core/os/memory.cpp:137
    #2 0xe581a4e in CowData<VMap<Object::Signal::Target, Object::Signal::Slot>::Pair>::resize(int) core/cowdata.h:283
    #3 0xe5812cb in CowData<VMap<Object::Signal::Target, Object::Signal::Slot>::Pair>::insert(int, VMap<Object::Signal::Target, Object::Signal::Slot>::Pair const&) core/cowdata.h:175
    #4 0xe57233a in VMap<Object::Signal::Target, Object::Signal::Slot>::insert(Object::Signal::Target const&, Object::Signal::Slot const&) core/vmap.h:125
    #5 0xe5688d5 in VMap<Object::Signal::Target, Object::Signal::Slot>::operator[](Object::Signal::Target const&) core/vmap.h:199
    #6 0xe549b1a in Object::connect(StringName const&, Object*, StringName const&, Vector<Variant> const&, unsigned int) core/object.cpp:1485
    #7 0xe595430 in MethodBind5R<Error, StringName const&, Object*, StringName const&, Vector<Variant> const&, unsigned int>::call(Object*, Variant const**, int, Variant::CallError&) core/method_bind.gen.inc:4177
    #8 0xe5373aa in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:921
    #9 0xe7a9e6e in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) core/variant_call.cpp:1112
    #10 0x1a1836a in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_function.cpp:1081
    #11 0x18549c3 in GDScriptInstance::call(StringName const&, Variant const**, int, Variant::CallError&) modules/gdscript/gdscript.cpp:1173
    #12 0xe536f17 in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:900
    #13 0xf057b55 in _Thread::_start_func(void*) core/bind/core_bind.cpp:2664
    #14 0x4f836b5 in ThreadPosix::thread_callback(void*) drivers/unix/thread_posix.cpp:74
    #15 0x7fc65bc18668 in start_thread /build/glibc-4WA41p/glibc-2.30/nptl/pthread_create.c:479

Thread T6 (_thread) created by T0 here:
    #0 0x7fc65c1b8805 in pthread_create (/lib/x86_64-linux-gnu/libasan.so.5+0x3a805)
    #1 0x4f83bb0 in ThreadPosix::create_func_posix(void (*)(void*), void*, Thread::Settings const&) drivers/unix/thread_posix.cpp:90
    #2 0xea7448f in Thread::create(void (*)(void*), void*, Thread::Settings const&) core/os/thread.cpp:51
    #3 0xf0595c1 in _Thread::start(Object*, StringName const&, Variant const&, _Thread::Priority) core/bind/core_bind.cpp:2710
    #4 0xf1305d1 in MethodBind4R<Error, Object*, StringName const&, Variant const&, _Thread::Priority>::call(Object*, Variant const**, int, Variant::CallError&) core/method_bind.gen.inc:3325
    #5 0xe5373aa in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:921
    #6 0xe7a9e6e in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) core/variant_call.cpp:1112
    #7 0x1a1836a in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_function.cpp:1081
    #8 0x1855714 in GDScriptInstance::_ml_call_reversed(GDScript*, StringName const&, Variant const**, int) modules/gdscript/gdscript.cpp:1204
    #9 0x18559c3 in GDScriptInstance::call_multilevel_reversed(StringName const&, Variant const**, int) modules/gdscript/gdscript.cpp:1211
    #10 0x92ef1cc in Node::_notification(int) scene/main/node.cpp:149
    #11 0x171077b in Node::_notificationv(int, bool) scene/main/node.h:46
    #12 0x1712c25 in CanvasItem::_notificationv(int, bool) scene/2d/canvas_item.h:166
    #13 0xacff0d7 in Node2D::_notificationv(int, bool) scene/2d/node_2d.h:38
    #14 0xe53783c in Object::notification(int, bool) core/object.cpp:931
    #15 0x92f1169 in Node::_propagate_ready() scene/main/node.cpp:196
    #16 0x92f09c0 in Node::_propagate_ready() scene/main/node.cpp:188
    #17 0x933df06 in Node::_set_tree(SceneTree*) scene/main/node.cpp:2562
    #18 0x94027ab in SceneTree::init() scene/main/scene_tree.cpp:465
    #19 0x1482c66 in OS_X11::run() platform/x11/os_x11.cpp:3242
    #20 0x13fe299 in main platform/x11/godot_x11.cpp:56
    #21 0x7fc65ad7e1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

Thread T7 (_thread) created by T0 here:
    #0 0x7fc65c1b8805 in pthread_create (/lib/x86_64-linux-gnu/libasan.so.5+0x3a805)
    #1 0x4f83bb0 in ThreadPosix::create_func_posix(void (*)(void*), void*, Thread::Settings const&) drivers/unix/thread_posix.cpp:90
    #2 0xea7448f in Thread::create(void (*)(void*), void*, Thread::Settings const&) core/os/thread.cpp:51
    #3 0xf0595c1 in _Thread::start(Object*, StringName const&, Variant const&, _Thread::Priority) core/bind/core_bind.cpp:2710
    #4 0xf1305d1 in MethodBind4R<Error, Object*, StringName const&, Variant const&, _Thread::Priority>::call(Object*, Variant const**, int, Variant::CallError&) core/method_bind.gen.inc:3325
    #5 0xe5373aa in Object::call(StringName const&, Variant const**, int, Variant::CallError&) core/object.cpp:921
    #6 0xe7a9e6e in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) core/variant_call.cpp:1112
    #7 0x1a1836a in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_function.cpp:1081
    #8 0x1855714 in GDScriptInstance::_ml_call_reversed(GDScript*, StringName const&, Variant const**, int) modules/gdscript/gdscript.cpp:1204
    #9 0x18559c3 in GDScriptInstance::call_multilevel_reversed(StringName const&, Variant const**, int) modules/gdscript/gdscript.cpp:1211
    #10 0x92ef1cc in Node::_notification(int) scene/main/node.cpp:149
    #11 0x171077b in Node::_notificationv(int, bool) scene/main/node.h:46
    #12 0x1712c25 in CanvasItem::_notificationv(int, bool) scene/2d/canvas_item.h:166
    #13 0xacff0d7 in Node2D::_notificationv(int, bool) scene/2d/node_2d.h:38
    #14 0xe53783c in Object::notification(int, bool) core/object.cpp:931
    #15 0x92f1169 in Node::_propagate_ready() scene/main/node.cpp:196
    #16 0x92f09c0 in Node::_propagate_ready() scene/main/node.cpp:188
    #17 0x933df06 in Node::_set_tree(SceneTree*) scene/main/node.cpp:2562
    #18 0x94027ab in SceneTree::init() scene/main/scene_tree.cpp:465
    #19 0x1482c66 in OS_X11::run() platform/x11/os_x11.cpp:3242
    #20 0x13fe299 in main platform/x11/godot_x11.cpp:56
    #21 0x7fc65ad7e1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

SUMMARY: AddressSanitizer: double-free (/lib/x86_64-linux-gnu/libasan.so.5+0x10df1e) in __interceptor_realloc