Make Servers truly Thread Safe #45852

reduz · 2021-02-09T16:26:20Z

-Rendering server now uses a split RID allocate/initialize internally, this allows generating RIDs immediately but initialization to happen later on the proper thread (as rendering APIs generally requiere to call on the right thread).
-RenderingServerWrapMT is no more, multithreading is done in RenderingServerDefault.
-Some functions like texture or mesh creation, when renderer supports it, can register and return immediately (so no waiting for server API to flush, and saving staging and command buffer memory).
-3D physics server changed to be made multithread friendly.
-Added PhysicsServer3DWrapMT to use 3D physics server from multiple threads.
-Disablet Bullet (too much effort to make multithread friendly, this needs to be fixed eventually).

Once merged, servers will be perfectly multithread-safe, so I can enable multthreaded loading by default, for editor and engine.

pouleyKetchoupp

Looks great! I haven't tested yet, but I've reviewed the physics code.

I haven't spotted any huge issue, just some details to fix or discuss. Most comments for 3D physics also apply for 2D.

servers/physics_3d/physics_server_3d_sw.cpp

servers/physics_server_3d.cpp

scene/3d/physics_joint_3d.cpp

servers/physics_server_3d.h

servers/physics_server_3d.cpp

servers/physics_3d/physics_server_3d_wrap_mt.h

qarmin · 2021-02-09T18:12:56Z

When enabling multithread rendering and running project - https://github.com/qarmin/RegressionTestProject/archive/4.0.zip
then I got this errors and address sanitizer log:

ERROR: Method/function failed.
   at: free (./core/templates/rid_owner.h:236)
=================================================================
==297548==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160006d7360 at pc 0x000011245b1c bp 0x7f0d1b4c5660 sp 0x7f0d1b4c5650
READ of size 8 at 0x6160006d7360 thread T11
ERROR: Invalid ID.
   at: free (servers/physics_3d/physics_server_3d_sw.cpp:1319)
ERROR: Invalid ID.
   at: free (servers/physics_3d/physics_server_3d_sw.cpp:1319)
ERROR: Invalid ID.
   at: free (servers/physics_3d/physics_server_3d_sw.cpp:1319)
ERROR: Invalid ID.
   at: free (servers/physics_3d/physics_server_3d_sw.cpp:1319)
ERROR: Invalid ID.
   at: free (servers/physics_3d/physics_server_3d_sw.cpp:1319)
    #0 0x11245b1b in RendererSceneCull::instance_set_base(RID, RID) servers/rendering/renderer_scene_cull.cpp:504
    #1 0x10535b97 in CommandQueueMT::Command2<RendererScene, void (RendererScene::*)(RID, RID), RID, RID>::call() core/templates/command_queue_mt.h:320
    #2 0xfce423c in CommandQueueMT::flush_one(bool) core/templates/command_queue_mt.h:461
    #3 0xfce4c69 in CommandQueueMT::wait_and_flush_one() core/templates/command_queue_mt.h:498
    #4 0x102832a8 in RenderingServerDefault::_thread_loop() servers/rendering/rendering_server_default.cpp:362
    #5 0x102829fb in RenderingServerDefault::_thread_callback(void*) servers/rendering/rendering_server_default.cpp:348
    #6 0x11ce993d in Thread::callback(Thread*, Thread::Settings const&, void (*)(void*), void*) core/os/thread.cpp:66
    #7 0x11cecdcd in void std::__invoke_impl<void, void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*>(std::__invoke_other, void (*&&)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*&&, Thread::Settings&&, void (*&&)(void*), void*&&) /usr/include/c++/10/bits/invoke.h:60
    #8 0x11cec69b in std::__invoke_result<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*>::type std::__invoke<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*>(void (*&&)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*&&, Thread::Settings&&, void (*&&)(void*), void*&&) /usr/include/c++/10/bits/invoke.h:95
    #9 0x11cec203 in void std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> >::_M_invoke<0ul, 1ul, 2ul, 3ul, 4ul>(std::_Index_tuple<0ul, 1ul, 2ul, 3ul, 4ul>) /usr/include/c++/10/thread:264
    #10 0x11cebe9a in std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> >::operator()() /usr/include/c++/10/thread:271
    #11 0x11cebe52 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> > >::_M_run() /usr/include/c++/10/thread:215
    #12 0x14378e63 in execute_native_thread_routine (/mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s+0x14378e63)
    #13 0x7f0d3ebbe608 in start_thread /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477
    #14 0x7f0d3e021292 in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x122292)

0x6160006d7360 is located 480 bytes inside of 568-byte region [0x6160006d7180,0x6160006d73b8)
freed by thread T0 here:
    #0 0x7f0d3ef8a1b7 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.6+0xb01b7)
    #1 0x11cd83d8 in Memory::free_static(void*, bool) core/os/memory.cpp:169
    #2 0x113257d0 in void memdelete<RendererSceneCull::Instance>(RendererSceneCull::Instance*) core/os/memory.h:118
    #3 0x112aa5ac in RendererSceneCull::free(RID) servers/rendering/renderer_scene_cull.cpp:3491
    #4 0x10277227 in RenderingServerDefault::free(RID) servers/rendering/rendering_server_default.cpp:77
    #5 0xd858799 in VisualInstance3D::~VisualInstance3D() scene/3d/visual_instance_3d.cpp:142
    #6 0xd321481 in Light3D::~Light3D() scene/3d/light_3d.cpp:359
    #7 0xd347180 in SpotLight3D::~SpotLight3D() scene/3d/light_3d.h:212
    #8 0x2bb66b9 in void memdelete<Node>(Node*) core/os/memory.h:115
    #9 0xbc7694a in Node::_notification(int) scene/main/node.cpp:169
    #10 0x2b730ef in Node::_notificationv(int, bool) scene/main/node.h:45
    #11 0x3d008ce in Node3D::_notificationv(int, bool) scene/3d/node_3d.h:52
    #12 0x12dd2bba in Object::notification(int, bool) core/object/object.cpp:793
    #13 0x12dc55f3 in Object::_predelete() core/object/object.cpp:354
    #14 0x12dfafc0 in predelete_handler(Object*) core/object/object.cpp:1824
    #15 0x1f2f85e in void memdelete<Object>(Object*) core/os/memory.h:111
    #16 0xbd74ad5 in SceneTree::_flush_delete_queue() scene/main/scene_tree.cpp:985
    #17 0xbd6376c in SceneTree::process(float) scene/main/scene_tree.cpp:453
    #18 0x1f252b4 in Main::iteration() main/main.cpp:2480
    #19 0x1dd55ea in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:261
    #20 0x1dc8520 in main platform/linuxbsd/godot_linuxbsd.cpp:58
    #21 0x7f0d3df260b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

previously allocated by thread T11 here:
    #0 0x7f0d3ef8a517 in malloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb0517)
    #1 0x11cd74cb in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:76
    #2 0x11cd73ca in operator new(unsigned long, char const*) core/os/memory.cpp:41
    #3 0x1123e7da in RendererSceneCull::instance_initialize(RID) servers/rendering/renderer_scene_cull.cpp:377
    #4 0x1053a1f4 in CommandQueueMT::Command1<RendererScene, void (RendererScene::*)(RID), RID>::call() core/templates/command_queue_mt.h:320
    #5 0xfce423c in CommandQueueMT::flush_one(bool) core/templates/command_queue_mt.h:461
    #6 0xfce4c69 in CommandQueueMT::wait_and_flush_one() core/templates/command_queue_mt.h:498
    #7 0x102832a8 in RenderingServerDefault::_thread_loop() servers/rendering/rendering_server_default.cpp:362
    #8 0x102829fb in RenderingServerDefault::_thread_callback(void*) servers/rendering/rendering_server_default.cpp:348
    #9 0x11ce993d in Thread::callback(Thread*, Thread::Settings const&, void (*)(void*), void*) core/os/thread.cpp:66
    #10 0x11cecdcd in void std::__invoke_impl<void, void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*>(std::__invoke_other, void (*&&)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*&&, Thread::Settings&&, void (*&&)(void*), void*&&) /usr/include/c++/10/bits/invoke.h:60
    #11 0x11cec69b in std::__invoke_result<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*>::type std::__invoke<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*>(void (*&&)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*&&, Thread::Settings&&, void (*&&)(void*), void*&&) /usr/include/c++/10/bits/invoke.h:95
    #12 0x11cec203 in void std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> >::_M_invoke<0ul, 1ul, 2ul, 3ul, 4ul>(std::_Index_tuple<0ul, 1ul, 2ul, 3ul, 4ul>) /usr/include/c++/10/thread:264
    #13 0x11cebe9a in std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> >::operator()() /usr/include/c++/10/thread:271
    #14 0x11cebe52 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> > >::_M_run() /usr/include/c++/10/thread:215
    #15 0x14378e63 in execute_native_thread_routine (/mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s+0x14378e63)

Thread T11 created by T0 here:
    #0 0x7f0d3ef31a95 in __interceptor_pthread_create (/lib/x86_64-linux-gnu/libasan.so.6+0x57a95)
    #1 0x14379128 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s+0x14379128)
    #2 0x11ce9dec in Thread::start(void (*)(void*), void*, Thread::Settings const&) core/os/thread.cpp:83
    #3 0x1027ffd2 in RenderingServerDefault::init() servers/rendering/rendering_server_default.cpp:237
    #4 0x1f030d5 in Main::setup2(unsigned long) main/main.cpp:1574
    #5 0x1efcc90 in Main::setup(char const*, int, char**, bool) main/main.cpp:1406
    #6 0x1dc8425 in main platform/linuxbsd/godot_linuxbsd.cpp:51
    #7 0x7f0d3df260b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: heap-use-after-free servers/rendering/renderer_scene_cull.cpp:504 in RendererSceneCull::instance_set_base(RID, RID)

qarmin · 2021-02-09T18:24:08Z

With default single threaded mode, it crashes in different place(both crashes doesn't occur in master Godot branch):

==302779==ERROR: AddressSanitizer: heap-use-after-free on address 0x61500014f090 at pc 0x00000fc7e9a3 bp 0x7ffeec4a6030 sp 0x7ffeec4a6020
READ of size 8 at 0x61500014f090 thread T0
    #0 0xfc7e9a2 in PhysicsServer3DSW::cone_twist_joint_set_param(RID, PhysicsServer3D::ConeTwistJointParam, float) servers/physics_3d/physics_server_3d_sw.cpp:1177
    #1 0xfd4eaa8 in PhysicsServer3DWrapMT::cone_twist_joint_set_param(RID, PhysicsServer3D::ConeTwistJointParam, float) servers/physics_3d/physics_server_3d_wrap_mt.h:369
    #2 0xd5b8c16 in ConeTwistJoint3D::_configure_joint(RID, PhysicsBody3D*, PhysicsBody3D*) scene/3d/physics_joint_3d.cpp:661
    #3 0xd588467 in Joint3D::_update_joint(bool) scene/3d/physics_joint_3d.cpp:127
    #4 0xd58bff0 in Joint3D::_notification(int) scene/3d/physics_joint_3d.cpp:195
    #5 0xd5f24e1 in Joint3D::_notificationv(int, bool) scene/3d/physics_joint_3d.h:38
    #6 0xd5fe132 in ConeTwistJoint3D::_notificationv(int, bool) scene/3d/physics_joint_3d.h:211
    #7 0x12dd2bba in Object::notification(int, bool) core/object/object.cpp:793
    #8 0xbc77e75 in Node::_propagate_ready() scene/main/node.cpp:188
    #9 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #10 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #11 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #12 0xbcc66fc in Node::_set_tree(SceneTree*) scene/main/node.cpp:2528
    #13 0xbc9b576 in Node::_add_child_nocheck(Node*, StringName const&) scene/main/node.cpp:1214
    #14 0xbc9cbe1 in Node::add_child(Node*, bool) scene/main/node.cpp:1232
    #15 0xa64b07b in void call_with_variant_args_helper<__UnexistingClass, Node*, bool, 0ul, 1ul>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool), Variant const**, Callable::CallError&, IndexSequence<0ul, 1ul>) core/variant/binder_common.h:196
    #16 0xa645e93 in void call_with_variant_args_dv<__UnexistingClass, Node*, bool>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool), Variant const**, int, Callable::CallError&, Vector<Variant> const&) core/variant/binder_common.h:313
    #17 0xa63632c in MethodBindT<Node*, bool>::call(Object*, Variant const**, int, Callable::CallError&) core/object/method_bind.h:282
    #18 0x528b271 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_vm.cpp:1476
    #19 0x4e0491d in GDScriptInstance::call(StringName const&, Variant const**, int, Callable::CallError&) modules/gdscript/gdscript.cpp:1544
    #20 0x12e3bcd5 in ScriptInstance::call(StringName const&, Variant const&, Variant const&, Variant const&, Variant const&, Variant const&) core/object/script_language.cpp:314
    #21 0xbc6ef29 in Node::_notification(int) scene/main/node.cpp:58
    #22 0x2b730ef in Node::_notificationv(int, bool) scene/main/node.h:45
    #23 0x2b759be in CanvasItem::_notificationv(int, bool) scene/main/canvas_item.h:164
    #24 0x2b780a8 in Control::_notificationv(int, bool) scene/gui/control.h:47
    #25 0x12dd2bba in Object::notification(int, bool) core/object/object.cpp:793
    #26 0xbd6fc09 in SceneTree::_notify_group_pause(StringName const&, int) scene/main/scene_tree.cpp:815
    #27 0xbd62d07 in SceneTree::process(float) scene/main/scene_tree.cpp:444
    #28 0x1f252b4 in Main::iteration() main/main.cpp:2480
    #29 0x1dd55ea in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:261
    #30 0x1dc8520 in main platform/linuxbsd/godot_linuxbsd.cpp:58
    #31 0x7f5c6c8800b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #32 0x1dc805d in _start (/mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s+0x1dc805d)

0x61500014f090 is located 16 bytes inside of 472-byte region [0x61500014f080,0x61500014f258)
freed by thread T0 here:
    #0 0x7f5c6d8e41b7 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.6+0xb01b7)
    #1 0x11cd83d8 in Memory::free_static(void*, bool) core/os/memory.cpp:169
    #2 0xfca975a in void memdelete<Joint3DSW>(Joint3DSW*) (/mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s+0xfca975a)
    #3 0xfc7e736 in PhysicsServer3DSW::joint_make_cone_twist(RID, RID, Transform const&, RID, Transform const&) servers/physics_3d/physics_server_3d_sw.cpp:1171
    #4 0xfd4e05e in PhysicsServer3DWrapMT::joint_make_cone_twist(RID, RID, Transform const&, RID, Transform const&) servers/physics_3d/physics_server_3d_wrap_mt.h:367
    #5 0xd5b886d in ConeTwistJoint3D::_configure_joint(RID, PhysicsBody3D*, PhysicsBody3D*) scene/3d/physics_joint_3d.cpp:659
    #6 0xd588467 in Joint3D::_update_joint(bool) scene/3d/physics_joint_3d.cpp:127
    #7 0xd58bff0 in Joint3D::_notification(int) scene/3d/physics_joint_3d.cpp:195
    #8 0xd5f24e1 in Joint3D::_notificationv(int, bool) scene/3d/physics_joint_3d.h:38
    #9 0xd5fe132 in ConeTwistJoint3D::_notificationv(int, bool) scene/3d/physics_joint_3d.h:211
    #10 0x12dd2bba in Object::notification(int, bool) core/object/object.cpp:793
    #11 0xbc77e75 in Node::_propagate_ready() scene/main/node.cpp:188
    #12 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #13 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #14 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #15 0xbcc66fc in Node::_set_tree(SceneTree*) scene/main/node.cpp:2528
    #16 0xbc9b576 in Node::_add_child_nocheck(Node*, StringName const&) scene/main/node.cpp:1214
    #17 0xbc9cbe1 in Node::add_child(Node*, bool) scene/main/node.cpp:1232
    #18 0xa64b07b in void call_with_variant_args_helper<__UnexistingClass, Node*, bool, 0ul, 1ul>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool), Variant const**, Callable::CallError&, IndexSequence<0ul, 1ul>) core/variant/binder_common.h:196
    #19 0xa645e93 in void call_with_variant_args_dv<__UnexistingClass, Node*, bool>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool), Variant const**, int, Callable::CallError&, Vector<Variant> const&) core/variant/binder_common.h:313
    #20 0xa63632c in MethodBindT<Node*, bool>::call(Object*, Variant const**, int, Callable::CallError&) core/object/method_bind.h:282
    #21 0x528b271 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_vm.cpp:1476
    #22 0x4e0491d in GDScriptInstance::call(StringName const&, Variant const**, int, Callable::CallError&) modules/gdscript/gdscript.cpp:1544
    #23 0x12e3bcd5 in ScriptInstance::call(StringName const&, Variant const&, Variant const&, Variant const&, Variant const&, Variant const&) core/object/script_language.cpp:314
    #24 0xbc6ef29 in Node::_notification(int) scene/main/node.cpp:58
    #25 0x2b730ef in Node::_notificationv(int, bool) scene/main/node.h:45
    #26 0x2b759be in CanvasItem::_notificationv(int, bool) scene/main/canvas_item.h:164
    #27 0x2b780a8 in Control::_notificationv(int, bool) scene/gui/control.h:47
    #28 0x12dd2bba in Object::notification(int, bool) core/object/object.cpp:793
    #29 0xbd6fc09 in SceneTree::_notify_group_pause(StringName const&, int) scene/main/scene_tree.cpp:815

previously allocated by thread T0 here:
    #0 0x7f5c6d8e4517 in malloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb0517)
    #1 0x11cd74cb in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:76
    #2 0x11cd73ca in operator new(unsigned long, char const*) core/os/memory.cpp:41
    #3 0xfc7e510 in PhysicsServer3DSW::joint_make_cone_twist(RID, RID, Transform const&, RID, Transform const&) servers/physics_3d/physics_server_3d_sw.cpp:1167
    #4 0xfd4e05e in PhysicsServer3DWrapMT::joint_make_cone_twist(RID, RID, Transform const&, RID, Transform const&) servers/physics_3d/physics_server_3d_wrap_mt.h:367
    #5 0xd5b886d in ConeTwistJoint3D::_configure_joint(RID, PhysicsBody3D*, PhysicsBody3D*) scene/3d/physics_joint_3d.cpp:659
    #6 0xd588467 in Joint3D::_update_joint(bool) scene/3d/physics_joint_3d.cpp:127
    #7 0xd58bff0 in Joint3D::_notification(int) scene/3d/physics_joint_3d.cpp:195
    #8 0xd5f24e1 in Joint3D::_notificationv(int, bool) scene/3d/physics_joint_3d.h:38
    #9 0xd5fe132 in ConeTwistJoint3D::_notificationv(int, bool) scene/3d/physics_joint_3d.h:211
    #10 0x12dd2bba in Object::notification(int, bool) core/object/object.cpp:793
    #11 0xbc77e75 in Node::_propagate_ready() scene/main/node.cpp:188
    #12 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #13 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #14 0xbc776db in Node::_propagate_ready() scene/main/node.cpp:180
    #15 0xbcc66fc in Node::_set_tree(SceneTree*) scene/main/node.cpp:2528
    #16 0xbc9b576 in Node::_add_child_nocheck(Node*, StringName const&) scene/main/node.cpp:1214
    #17 0xbc9cbe1 in Node::add_child(Node*, bool) scene/main/node.cpp:1232
    #18 0xa64b07b in void call_with_variant_args_helper<__UnexistingClass, Node*, bool, 0ul, 1ul>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool), Variant const**, Callable::CallError&, IndexSequence<0ul, 1ul>) core/variant/binder_common.h:196
    #19 0xa645e93 in void call_with_variant_args_dv<__UnexistingClass, Node*, bool>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool), Variant const**, int, Callable::CallError&, Vector<Variant> const&) core/variant/binder_common.h:313
    #20 0xa63632c in MethodBindT<Node*, bool>::call(Object*, Variant const**, int, Callable::CallError&) core/object/method_bind.h:282
    #21 0x528b271 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_vm.cpp:1476
    #22 0x4e0491d in GDScriptInstance::call(StringName const&, Variant const**, int, Callable::CallError&) modules/gdscript/gdscript.cpp:1544
    #23 0x12e3bcd5 in ScriptInstance::call(StringName const&, Variant const&, Variant const&, Variant const&, Variant const&, Variant const&) core/object/script_language.cpp:314
    #24 0xbc6ef29 in Node::_notification(int) scene/main/node.cpp:58
    #25 0x2b730ef in Node::_notificationv(int, bool) scene/main/node.h:45
    #26 0x2b759be in CanvasItem::_notificationv(int, bool) scene/main/canvas_item.h:164
    #27 0x2b780a8 in Control::_notificationv(int, bool) scene/gui/control.h:47
    #28 0x12dd2bba in Object::notification(int, bool) core/object/object.cpp:793
    #29 0xbd6fc09 in SceneTree::_notify_group_pause(StringName const&, int) scene/main/scene_tree.cpp:815

SUMMARY: AddressSanitizer: heap-use-after-free servers/physics_3d/physics_server_3d_sw.cpp:1177 in PhysicsServer3DSW::cone_twist_joint_set_param(RID, PhysicsServer3D::ConeTwistJointParam, float)

reduz · 2021-02-09T20:54:21Z

@quarmin I think it should be fixed with latest commit, but let me know just in case

reduz · 2021-02-09T21:05:40Z

There is still one more thing I have to do, which is to refactor the command queue for all this to actually work properly, but I will do it in the next PR.

qarmin · 2021-02-09T21:52:27Z

Crash with multithread rendering still exists(this one with physics was fixed). - RegressionTestProject.zip

Now additionally prints this lines

servers/rendering/renderer_scene_cull.cpp:440:32: runtime error: downcast of address 0x608001087fb0 which does not point to an object of type 'InstanceLightData'
0x608001087fb0: note: object is of type 'RendererSceneCull::InstanceBaseData'
 be be be be  90 6d 86 1d 00 00 00 00  0e 00 00 00 a3 50 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'RendererSceneCull::InstanceBaseData'

RandomShaper

static_cast<PhysicsServer3DSW *>(PhysicsServer3D::get_singleton())
must be replaced by
PhysicsServer3DSW::singleton

RandomShaper · 2021-02-09T23:51:24Z

@qarmin, cold you try the same (with multi-threaded rendering) with this patch applied?
safe_rids_patch.txt

I have the suspicion that some RID_Owners need to be thread-safe.

reduz · 2021-02-10T00:49:29Z

@RandomShaper ah man, I forgot to make those thread safe, great thing you spotted it

reduz · 2021-02-10T01:15:12Z

@RandomShaper @qarmin ok, should be fixed now.

It will probably still error (needs the rewrite of CommandQueue to truly work as intended), but should not crash.

qarmin · 2021-02-10T06:42:14Z

I still have same crash
A lot of smaller project - RegressionTestProject.zip

It only adds nodes to scene and later remove them

reduz · 2021-02-10T10:57:21Z

@qarmin this seems like a bug in the command queue that I don't understand, as it looks server code is being called from different threads (creating a race condition, or that's what it looks like from debugging) when it should not, so it's a bit unrelated to this PR. Currently @hpvb is rewriting the command queue, and I suggest testing this reproduction case against his new work (or opening an issue and we make sure his PR passes it).

RandomShaper · 2021-02-10T12:00:00Z

@reduz, the last version pushed crashes for me unless I make more RID_Owners thread safe.

Specifically, I believe that every RID_Owner whose contents are subject to the allocate-initialize split need to be thread-safe, since they are accessed by both the server thread and any thread creating an object.

I've changed all the RID_Owners to thread-safe locally (as I did in the patch I posted) and the crash is gone.

Not sure if that's something that will be addressed by the command queue rewrite.

RandomShaper · 2021-02-10T12:04:49Z

Sorry, I approved by mistake. I didn't want to approve yet. I've re-asked myself for review.

reduz · 2021-02-10T13:08:04Z

@RandomShaper I already changed the rid owners to thread safe in my latest forced push, not sure which ones you believe are missing.

modules/bullet/config.py

-Rendering server now uses a split RID allocate/initialize internally, this allows generating RIDs immediately but initialization to happen later on the proper thread (as rendering APIs generally requiere to call on the right thread). -RenderingServerWrapMT is no more, multithreading is done in RenderingServerDefault. -Some functions like texture or mesh creation, when renderer supports it, can register and return immediately (so no waiting for server API to flush, and saving staging and command buffer memory). -3D physics server changed to be made multithread friendly. -Added PhysicsServer3DWrapMT to use 3D physics server from multiple threads. -Disablet Bullet (too much effort to make multithread friendly, this needs to be fixed eventually).

Changes done.

akien-mga · 2021-02-10T16:39:56Z

Thanks!

It has been disabled in `master` since one year (godotengine#45852) and our plan is for Bullet, and possibly other thirdparty physics engines, to be implemented via GDExtension so that they can be selected by the users who need them.

reduz requested a review from AndreaCatania as a code owner February 9, 2021 16:26

pouleyKetchoupp requested review from a team and removed request for AndreaCatania February 9, 2021 16:30

pouleyKetchoupp added topic:core enhancement labels Feb 9, 2021

pouleyKetchoupp added this to the 4.0 milestone Feb 9, 2021

akien-mga requested review from a team February 9, 2021 16:34

pouleyKetchoupp previously requested changes Feb 9, 2021

View reviewed changes

reduz force-pushed the make-servers-truly-thread-safe branch 2 times, most recently from 52bdfee to 5f1ec4b Compare February 9, 2021 20:53

reduz requested a review from vnen as a code owner February 9, 2021 20:53

RandomShaper requested changes Feb 9, 2021

View reviewed changes

reduz force-pushed the make-servers-truly-thread-safe branch from 5f1ec4b to 66388b9 Compare February 10, 2021 01:14

reduz force-pushed the make-servers-truly-thread-safe branch from 66388b9 to 1bb732e Compare February 10, 2021 11:08

RandomShaper approved these changes Feb 10, 2021

View reviewed changes

RandomShaper self-requested a review February 10, 2021 12:03

akien-mga reviewed Feb 10, 2021

View reviewed changes

modules/bullet/config.py Show resolved Hide resolved

reduz force-pushed the make-servers-truly-thread-safe branch from bbf9994 to 8b19ffd Compare February 10, 2021 16:22

akien-mga approved these changes Feb 10, 2021

View reviewed changes

akien-mga merged commit 1808f1d into godotengine:master Feb 10, 2021

pouleyKetchoupp mentioned this pull request Feb 10, 2021

Cylinder support in Godot Physics 3D #45854

Merged

RandomShaper mentioned this pull request Feb 10, 2021

Add RID reservation infrastructure #45064

Closed

pouleyKetchoupp mentioned this pull request Feb 18, 2021

PhysicsServer3D errors when starting new project #46167

Closed

Calinou mentioned this pull request Feb 27, 2021

Undocumented thread (un)safety #20964

Open

pouleyKetchoupp mentioned this pull request Mar 21, 2021

Fix KinematicBody3D having a duplicate locked_axis variable #45175

Closed

This was referenced Apr 18, 2021

Test Godot with Vulkan in CI #47414

Merged

Revert "Test Godot with Vulkan in CI" #48024

Closed

Hoimar mentioned this pull request May 1, 2021

Planetary collisions Hoimar/Planet-Generator#1

Open

HaSa1002 mentioned this pull request Jun 9, 2021

Preloading a scene containing materials causes deadlock in SpinLock through material_owner.owns(p_rid) in RendererStorageRD::free #49201

Closed

pouleyKetchoupp mentioned this pull request Aug 24, 2021

[TRACKER] Bullet Physics issues #45022

Open

pouleyKetchoupp mentioned this pull request Oct 7, 2021

Crash in BVH when using move_and_slide or move_and_collide with multi-threaded physics server #53496

Closed

rburing mentioned this pull request Feb 27, 2022

Joint2D and Joint3D: disconnect only connected signals #58596

Merged

akien-mga mentioned this pull request Mar 9, 2022

Remove unused Bullet module and thirdparty code #58946

Merged

dzil123 mentioned this pull request Sep 24, 2022

World3D.direct_space_state add multithreaded warning #66348

Merged

KoBeWi mentioned this pull request Aug 19, 2023

Threaded resource loading can still cause stutters #80770

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Servers truly Thread Safe #45852

Make Servers truly Thread Safe #45852

reduz commented Feb 9, 2021

pouleyKetchoupp left a comment

qarmin commented Feb 9, 2021

qarmin commented Feb 9, 2021

reduz commented Feb 9, 2021

reduz commented Feb 9, 2021

qarmin commented Feb 9, 2021

RandomShaper left a comment

RandomShaper commented Feb 9, 2021

reduz commented Feb 10, 2021

reduz commented Feb 10, 2021

qarmin commented Feb 10, 2021

reduz commented Feb 10, 2021 •

edited

Loading

RandomShaper commented Feb 10, 2021 •

edited

Loading

RandomShaper commented Feb 10, 2021

reduz commented Feb 10, 2021

akien-mga commented Feb 10, 2021

Make Servers truly Thread Safe #45852

Make Servers truly Thread Safe #45852

Conversation

reduz commented Feb 9, 2021

pouleyKetchoupp left a comment

Choose a reason for hiding this comment

qarmin commented Feb 9, 2021

qarmin commented Feb 9, 2021

reduz commented Feb 9, 2021

reduz commented Feb 9, 2021

qarmin commented Feb 9, 2021

RandomShaper left a comment

Choose a reason for hiding this comment

RandomShaper commented Feb 9, 2021

reduz commented Feb 10, 2021

reduz commented Feb 10, 2021

qarmin commented Feb 10, 2021

reduz commented Feb 10, 2021 • edited Loading

RandomShaper commented Feb 10, 2021 • edited Loading

RandomShaper commented Feb 10, 2021

reduz commented Feb 10, 2021

akien-mga commented Feb 10, 2021

reduz commented Feb 10, 2021 •

edited

Loading

RandomShaper commented Feb 10, 2021 •

edited

Loading