Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data races when running Godot #32081

Open
qarmin opened this issue Sep 10, 2019 · 10 comments
Open

Data races when running Godot #32081

qarmin opened this issue Sep 10, 2019 · 10 comments
Assignees
Milestone

Comments

@qarmin
Copy link
Contributor

qarmin commented Sep 10, 2019

Godot version:
3.2.alpha.custom_build. 24e1039

OS/device including version:
Ubuntu 19.04

Issue description:
When I run Godot project manager, then thread sanitizer shows some data races(reading data in one thread, when saving in another thread)

WARNING: ThreadSanitizer: data race (pid=27914)
  Atomic write of size 8 at 0x0000092a1ec0 by thread T1:
    #0 __tsan_atomic64_fetch_add <null> (godot.x11.tools.64.llvms+0x1703076)
    #1 unsigned long atomic_add<unsigned long, unsigned long>(unsigned long volatile*, unsigned long) /home/rafal/Pulpit/mojgodot/./core/safe_refcount.h:136:9 (godot.x11.tools.64.llvms+0x666442b)
    #2 Memory::alloc_static(unsigned long, bool) /home/rafal/Pulpit/mojgodot/core/os/memory.cpp:98 (godot.x11.tools.64.llvms+0x666442b)
    #3 operator new(unsigned long, char const*) /home/rafal/Pulpit/mojgodot/core/os/memory.cpp:42:9 (godot.x11.tools.64.llvms+0x6664268)
    #4 ThreadPosix::thread_callback(void*) /home/rafal/Pulpit/mojgodot/drivers/unix/thread_posix.cpp:70:45 (godot.x11.tools.64.llvms+0x2d3e899)

  Previous read of size 8 at 0x0000092a1ec0 by main thread:
    #0 Memory::alloc_static(unsigned long, bool) /home/rafal/Pulpit/mojgodot/core/os/memory.cpp:99:42 (godot.x11.tools.64.llvms+0x6664441)
    #1 CowData<wchar_t>::resize(int) /home/rafal/Pulpit/mojgodot/./core/cowdata.h:274:32 (godot.x11.tools.64.llvms+0x22ceb5b)
    #2 String::resize(int) /home/rafal/Pulpit/mojgodot/./core/ustring.h:154:45 (godot.x11.tools.64.llvms+0x2efe9d4)
    #3 String::copy_from(char const*) /home/rafal/Pulpit/mojgodot/core/ustring.cpp:169:2 (godot.x11.tools.64.llvms+0x64bc814)
    #4 String::operator=(char const*) /home/rafal/Pulpit/mojgodot/core/ustring.cpp:331:2 (godot.x11.tools.64.llvms+0x64bd758)
    #5 detect_prime() /home/rafal/Pulpit/mojgodot/platform/x11/detect_prime.cpp:135:15 (godot.x11.tools.64.llvms+0x1782c12)
    #6 OS_X11::initialize(OS::VideoMode const&, int, int) /home/rafal/Pulpit/mojgodot/platform/x11/os_x11.cpp:275:16 (godot.x11.tools.64.llvms+0x174b148)
    #7 Main::setup2(unsigned long) /home/rafal/Pulpit/mojgodot/main/main.cpp:1117:35 (godot.x11.tools.64.llvms+0x17bade5)
    #8 Main::setup(char const*, int, char**, bool) /home/rafal/Pulpit/mojgodot/main/main.cpp:1062:10 (godot.x11.tools.64.llvms+0x17b9f28)
    #9 main /home/rafal/Pulpit/mojgodot/platform/x11/godot_x11.cpp:49:14 (godot.x11.tools.64.llvms+0x1744730)

  Location is global 'Memory::mem_usage' of size 8 at 0x0000092a1ec0 (godot.x11.tools.64.llvms+0x0000092a1ec0)

  Thread T1 (tid=27916, running) created by main thread at:
    #0 pthread_create <null> (godot.x11.tools.64.llvms+0x16b90d5)
    #1 ThreadPosix::create_func_posix(void (*)(void*), void*, Thread::Settings const&) /home/rafal/Pulpit/mojgodot/drivers/unix/thread_posix.cpp:90:2 (godot.x11.tools.64.llvms+0x2d3eadf)
    #2 Thread::create(void (*)(void*), void*, Thread::Settings const&) /home/rafal/Pulpit/mojgodot/core/os/thread.cpp:51:10 (godot.x11.tools.64.llvms+0x666c8b0)
    #3 IP::IP() /home/rafal/Pulpit/mojgodot/core/io/ip.cpp:326:22 (godot.x11.tools.64.llvms+0x679c0df)
    #4 IP_Unix::IP_Unix() /home/rafal/Pulpit/mojgodot/drivers/unix/ip_unix.cpp:265:10 (godot.x11.tools.64.llvms+0x2f208dc)
    #5 IP_Unix::_create_unix() /home/rafal/Pulpit/mojgodot/drivers/unix/ip_unix.cpp:262:9 (godot.x11.tools.64.llvms+0x2f2085b)
    #6 IP::create() /home/rafal/Pulpit/mojgodot/core/io/ip.cpp:310:9 (godot.x11.tools.64.llvms+0x679bde3)
    #7 register_core_types() /home/rafal/Pulpit/mojgodot/core/register_core_types.cpp:211:7 (godot.x11.tools.64.llvms+0x641b468)
    #8 Main::setup(char const*, int, char**, bool) /home/rafal/Pulpit/mojgodot/main/main.cpp:341:2 (godot.x11.tools.64.llvms+0x17abf12)
    #9 main /home/rafal/Pulpit/mojgodot/platform/x11/godot_x11.cpp:49:14 (godot.x11.tools.64.llvms+0x1744730)

SUMMARY: ThreadSanitizer: data race (/home/rafal/Pulpit/mojgodot/bin/godot.x11.tools.64.llvms+0x1703076) in __tsan_atomic64_fetch_add

Steps to reproduce:

  1. Compile Godot with thread sanitizer support e.g.(newer versions of GCC compiler are a little buggy for now, so llvm is the only option)
scons p=x11 -j6 use_tsan=yes use_llvm=yes
  1. Run project manager
bin/godot.x11.tools.64.llvms
@akien-mga akien-mga added this to the 3.2 milestone Sep 23, 2019
@pwaller
Copy link

pwaller commented Dec 29, 2019

There are lots of these errors, many of them surround the refcount primitive.

I recommend reading this excellent article for some background on data races, and to understand why even "benign-looking" code often isn't benign: https://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong.

@pwaller
Copy link

pwaller commented Dec 29, 2019

Here is also another great reference from the thread sanitizer docs: https://github.com/google/sanitizers/wiki/ThreadSanitizerPopularDataRaces

@qarmin
Copy link
Contributor Author

qarmin commented Jul 5, 2020

When running my game with Godot 3.2.2, there is a lot of more errors - Thread Sanitizer Log.txt

Strange that this happens a lot with SafeRefCount::unref() / ::ref() which as name suggest, should be safe because operate on atomic_increment and decrement

@pwaller
Copy link

pwaller commented Jul 6, 2020

Strange that this happens a lot with SafeRefCount::unref() / ::ref()

One issue there is that both the read and the write must be atomic or otherwise synchronized. See the language in the error messages, e.g. ("Atomic write" vs "previous read").

One of the warnings indicates for example that atomic_conditional_increment is actually doing a non-atomic read of the memory it just incremented (return tmp + 1 does a non-atomic read of tmp which is a reference to the atomically-incremented variable):

godot/core/safe_refcount.h

Lines 101 to 109 in b96b0d9

while (true) {
T tmp = static_cast<T const volatile &>(*pw);
if (tmp == 0) {
return 0; // if zero, can't add to it anymore
}
if (__sync_val_compare_and_swap(pw, tmp, tmp + 1) == tmp) {
return tmp + 1;
}
}

I refer also again to https://software.intel.com/content/www/us/en/develop/blogs/benign-data-races-what-could-possibly-go-wrong.html -- I think this is actually one of the examples given in that blog post of a "what could possibly go wrong", at the end.

@RandomShaper
Copy link
Member

I have plans to address much of these issues very soon. I'll fix all the wrong approaches to atomicity in 4.0 and then port them to 3.2.

My expectation is that we'll get rid of a number of hard to debug issues that stem from that.

Please stay tuned. 😄

@qarmin
Copy link
Contributor Author

qarmin commented Dec 25, 2020

When running https://github.com/qarmin/RegressionTestProject/tree/3.2, then there is still huge amount of data races - 7_Use Godot.txt in 3.2.4.beta.custom_build. ebe9d61

@qarmin
Copy link
Contributor Author

qarmin commented Jul 27, 2021

Thread sanitizer still shows with 4.0 2fa4b59 e.g. lock-order-inversion when opening and closing editor - log.txt

@akien-mga
Copy link
Member

Is this still reproducible in latest 4.x builds?

@akien-mga akien-mga modified the milestones: 4.0, 4.x Feb 26, 2023
@rcorre
Copy link
Contributor

rcorre commented Apr 29, 2023

After patching around #76581, I see a lot of lock-order-inversion warnings in 4.0.2:
tsan.txt

==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=80028)
  Cycle in lock order graph: M0 (0x7b5000044a10) => M1 (0x7b8800022448) => M0

  Mutex M1 acquired here while holding mutex M0 in main thread:
    #0 pthread_mutex_lock <null> (godot.linuxbsd.editor.x86_64.llvm.san+0x2e12851) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:749:12 (godot.linuxbsd.editor.x86_64.llvm.san+0x45c5969) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #2 __gthread_recursive_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:811:10 (godot.linuxbsd.editor.x86_64.llvm.san+0x45c5969)
    #3 std::recursive_mutex::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/mutex:108:17 (godot.linuxbsd.editor.x86_64.llvm.san+0x45c5969)
    #4 std::unique_lock<std::recursive_mutex>::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/unique_lock.h:139:17 (godot.linuxbsd.editor.x86_64.llvm.san+0x45c5969)
    #5 std::unique_lock<std::recursive_mutex>::unique_lock(std::recursive_mutex&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/unique_lock.h:69:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45c5969)
    #6 MutexLock<MutexImpl<std::recursive_mutex>>::MutexLock(MutexImpl<std::recursive_mutex> const&) /home/rcorre/src/godot/godot/./core/os/mutex.h:122:4 (godot.linuxbsd.editor.x86_64.llvm.san+0x45c5969)
    #7 TextServerAdvanced::_shaped_text_shape(RID const&) /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.cpp:5543:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45c5969)
    #8 TextServerAdvanced::_shaped_text_update_breaks(RID const&) /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.cpp:4655:3 (godot.linuxbsd.editor.x86_64.llvm.san+0x45af220) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #9 TextServerAdvanced::shaped_text_update_breaks(RID const&) /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.h:890:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45e14ba) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #10 TextServer::shaped_text_get_line_breaks(RID const&, double, long, BitField<TextServer::LineBreakFlag>) const /home/rcorre/src/godot/godot/servers/text_server.cpp:809:34 (godot.linuxbsd.editor.x86_64.llvm.san+0x8432754) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #11 TextServerExtension::shaped_text_get_line_breaks(RID const&, double, long, BitField<TextServer::LineBreakFlag>) const /home/rcorre/src/godot/godot/servers/text/text_server_extension.cpp:1157:21 (godot.linuxbsd.editor.x86_64.llvm.san+0x8f5da82) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #12 Label::_shape() /home/rcorre/src/godot/godot/scene/gui/label.cpp:148:38 (godot.linuxbsd.editor.x86_64.llvm.san+0x6c8c7a5) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #13 Label::get_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/label.cpp:619:30 (godot.linuxbsd.editor.x86_64.llvm.san+0x6c9366b) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #14 Control::_update_minimum_size_cache() /home/rcorre/src/godot/godot/scene/gui/control.cpp:1608:18 (godot.linuxbsd.editor.x86_64.llvm.san+0x6b69405) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #15 Control::get_combined_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/control.cpp:1627:32 (godot.linuxbsd.editor.x86_64.llvm.san+0x6b69405)
    #16 AcceptDialog::_get_contents_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/dialogs.cpp:260:28 (godot.linuxbsd.editor.x86_64.llvm.san+0x6bb809a) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #17 Window::get_contents_minimum_size() const /home/rcorre/src/godot/godot/scene/main/window.cpp:1566:9 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #18 Window::get_clamped_minimum_size() const /home/rcorre/src/godot/godot/scene/main/window.cpp:1574:22 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c)
    #19 Window::_update_window_size() /home/rcorre/src/godot/godot/scene/main/window.cpp:883:22 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c)
    #20 Window::set_min_size(Vector2i const&) /home/rcorre/src/godot/godot/scene/main/window.cpp:391:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3cac4) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #21 ConfirmationDialog::ConfirmationDialog() /home/rcorre/src/godot/godot/scene/gui/dialogs.cpp:447:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x6bbcff1) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #22 EditorFileDialog::EditorFileDialog() /home/rcorre/src/godot/godot/editor/editor_file_dialog.cpp:1704:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x5263f09) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #23 EditorAssetLibrary::EditorAssetLibrary(bool) /home/rcorre/src/godot/godot/editor/plugins/asset_library_editor_plugin.cpp:1622:15 (godot.linuxbsd.editor.x86_64.llvm.san+0x5e7a7b9) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #24 ProjectManager::ProjectManager() /home/rcorre/src/godot/godot/editor/project_manager.cpp:2924:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x58d8e94) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #25 Main::start() /home/rcorre/src/godot/godot/main/main.cpp:3002:31 (godot.linuxbsd.editor.x86_64.llvm.san+0x2f2b926) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #26 main /home/rcorre/src/godot/godot/platform/linuxbsd/godot_linuxbsd.cpp:71:6 (godot.linuxbsd.editor.x86_64.llvm.san+0x2e67e41) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)

  Mutex M0 previously acquired by the same thread here:
    #0 pthread_mutex_lock <null> (godot.linuxbsd.editor.x86_64.llvm.san+0x2e12851) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:749:12 (godot.linuxbsd.editor.x86_64.llvm.san+0x45af1e6) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #2 __gthread_recursive_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:811:10 (godot.linuxbsd.editor.x86_64.llvm.san+0x45af1e6)
    #3 std::recursive_mutex::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/mutex:108:17 (godot.linuxbsd.editor.x86_64.llvm.san+0x45af1e6)
    #4 std::unique_lock<std::recursive_mutex>::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/unique_lock.h:139:17 (godot.linuxbsd.editor.x86_64.llvm.san+0x45af1e6)
    #5 std::unique_lock<std::recursive_mutex>::unique_lock(std::recursive_mutex&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/unique_lock.h:69:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45af1e6)
    #6 MutexLock<MutexImpl<std::recursive_mutex>>::MutexLock(MutexImpl<std::recursive_mutex> const&) /home/rcorre/src/godot/godot/./core/os/mutex.h:122:4 (godot.linuxbsd.editor.x86_64.llvm.san+0x45af1e6)
    #7 TextServerAdvanced::_shaped_text_update_breaks(RID const&) /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.cpp:4653:12 (godot.linuxbsd.editor.x86_64.llvm.san+0x45af1e6)
    #8 TextServerAdvanced::shaped_text_update_breaks(RID const&) /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.h:890:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45e14ba) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #9 TextServer::shaped_text_get_line_breaks(RID const&, double, long, BitField<TextServer::LineBreakFlag>) const /home/rcorre/src/godot/godot/servers/text_server.cpp:809:34 (godot.linuxbsd.editor.x86_64.llvm.san+0x8432754) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #10 TextServerExtension::shaped_text_get_line_breaks(RID const&, double, long, BitField<TextServer::LineBreakFlag>) const /home/rcorre/src/godot/godot/servers/text/text_server_extension.cpp:1157:21 (godot.linuxbsd.editor.x86_64.llvm.san+0x8f5da82) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #11 Label::_shape() /home/rcorre/src/godot/godot/scene/gui/label.cpp:148:38 (godot.linuxbsd.editor.x86_64.llvm.san+0x6c8c7a5) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #12 Label::get_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/label.cpp:619:30 (godot.linuxbsd.editor.x86_64.llvm.san+0x6c9366b) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #13 Control::_update_minimum_size_cache() /home/rcorre/src/godot/godot/scene/gui/control.cpp:1608:18 (godot.linuxbsd.editor.x86_64.llvm.san+0x6b69405) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #14 Control::get_combined_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/control.cpp:1627:32 (godot.linuxbsd.editor.x86_64.llvm.san+0x6b69405)
    #15 AcceptDialog::_get_contents_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/dialogs.cpp:260:28 (godot.linuxbsd.editor.x86_64.llvm.san+0x6bb809a) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #16 Window::get_contents_minimum_size() const /home/rcorre/src/godot/godot/scene/main/window.cpp:1566:9 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #17 Window::get_clamped_minimum_size() const /home/rcorre/src/godot/godot/scene/main/window.cpp:1574:22 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c)
    #18 Window::_update_window_size() /home/rcorre/src/godot/godot/scene/main/window.cpp:883:22 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c)
    #19 Window::set_min_size(Vector2i const&) /home/rcorre/src/godot/godot/scene/main/window.cpp:391:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3cac4) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #20 ConfirmationDialog::ConfirmationDialog() /home/rcorre/src/godot/godot/scene/gui/dialogs.cpp:447:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x6bbcff1) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #21 EditorFileDialog::EditorFileDialog() /home/rcorre/src/godot/godot/editor/editor_file_dialog.cpp:1704:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x5263f09) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #22 EditorAssetLibrary::EditorAssetLibrary(bool) /home/rcorre/src/godot/godot/editor/plugins/asset_library_editor_plugin.cpp:1622:15 (godot.linuxbsd.editor.x86_64.llvm.san+0x5e7a7b9) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #23 ProjectManager::ProjectManager() /home/rcorre/src/godot/godot/editor/project_manager.cpp:2924:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x58d8e94) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #24 Main::start() /home/rcorre/src/godot/godot/main/main.cpp:3002:31 (godot.linuxbsd.editor.x86_64.llvm.san+0x2f2b926) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #25 main /home/rcorre/src/godot/godot/platform/linuxbsd/godot_linuxbsd.cpp:71:6 (godot.linuxbsd.editor.x86_64.llvm.san+0x2e67e41) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)

  Mutex M0 acquired here while holding mutex M1 in main thread:
    #0 pthread_mutex_lock <null> (godot.linuxbsd.editor.x86_64.llvm.san+0x2e12851) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:749:12 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a98ce) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #2 __gthread_recursive_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:811:10 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a98ce)
    #3 std::recursive_mutex::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/mutex:108:17 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a98ce)
    #4 std::unique_lock<std::recursive_mutex>::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/unique_lock.h:139:17 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a98ce)
    #5 std::unique_lock<std::recursive_mutex>::unique_lock(std::recursive_mutex&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/unique_lock.h:69:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a98ce)
    #6 MutexLock<MutexImpl<std::recursive_mutex>>::MutexLock(MutexImpl<std::recursive_mutex> const&) /home/rcorre/src/godot/godot/./core/os/mutex.h:122:4 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a98ce)
    #7 TextServerAdvanced::_shaped_text_substr(RID const&, long, long) const /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.cpp:4089:12 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a98ce)
    #8 TextServerAdvanced::shaped_text_substr(RID const&, long, long) const /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.h:883:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45e12da) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #9 Label::_shape() /home/rcorre/src/godot/godot/scene/gui/label.cpp:150:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x6c8c905) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #10 Label::get_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/label.cpp:619:30 (godot.linuxbsd.editor.x86_64.llvm.san+0x6c9366b) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #11 Control::_update_minimum_size_cache() /home/rcorre/src/godot/godot/scene/gui/control.cpp:1608:18 (godot.linuxbsd.editor.x86_64.llvm.san+0x6b69405) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #12 Control::get_combined_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/control.cpp:1627:32 (godot.linuxbsd.editor.x86_64.llvm.san+0x6b69405)
    #13 AcceptDialog::_get_contents_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/dialogs.cpp:260:28 (godot.linuxbsd.editor.x86_64.llvm.san+0x6bb809a) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #14 Window::get_contents_minimum_size() const /home/rcorre/src/godot/godot/scene/main/window.cpp:1566:9 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #15 Window::get_clamped_minimum_size() const /home/rcorre/src/godot/godot/scene/main/window.cpp:1574:22 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c)
    #16 Window::_update_window_size() /home/rcorre/src/godot/godot/scene/main/window.cpp:883:22 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c)
    #17 Window::set_min_size(Vector2i const&) /home/rcorre/src/godot/godot/scene/main/window.cpp:391:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3cac4) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #18 ConfirmationDialog::ConfirmationDialog() /home/rcorre/src/godot/godot/scene/gui/dialogs.cpp:447:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x6bbcff1) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #19 EditorFileDialog::EditorFileDialog() /home/rcorre/src/godot/godot/editor/editor_file_dialog.cpp:1704:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x5263f09) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #20 EditorAssetLibrary::EditorAssetLibrary(bool) /home/rcorre/src/godot/godot/editor/plugins/asset_library_editor_plugin.cpp:1622:15 (godot.linuxbsd.editor.x86_64.llvm.san+0x5e7a7b9) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #21 ProjectManager::ProjectManager() /home/rcorre/src/godot/godot/editor/project_manager.cpp:2924:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x58d8e94) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #22 Main::start() /home/rcorre/src/godot/godot/main/main.cpp:3002:31 (godot.linuxbsd.editor.x86_64.llvm.san+0x2f2b926) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #23 main /home/rcorre/src/godot/godot/platform/linuxbsd/godot_linuxbsd.cpp:71:6 (godot.linuxbsd.editor.x86_64.llvm.san+0x2e67e41) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)

  Mutex M1 previously acquired by the same thread here:
    #0 pthread_mutex_lock <null> (godot.linuxbsd.editor.x86_64.llvm.san+0x2e12851) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:749:12 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a97e7) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #2 __gthread_recursive_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:811:10 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a97e7)
    #3 std::recursive_mutex::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/mutex:108:17 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a97e7)
    #4 std::unique_lock<std::recursive_mutex>::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/unique_lock.h:139:17 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a97e7)
    #5 std::unique_lock<std::recursive_mutex>::unique_lock(std::recursive_mutex&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/unique_lock.h:69:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a97e7)
    #6 MutexLock<MutexImpl<std::recursive_mutex>>::MutexLock(MutexImpl<std::recursive_mutex> const&) /home/rcorre/src/godot/godot/./core/os/mutex.h:122:4 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a97e7)
    #7 TextServerAdvanced::_shaped_text_substr(RID const&, long, long) const /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.cpp:4085:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45a97e7)
    #8 TextServerAdvanced::shaped_text_substr(RID const&, long, long) const /home/rcorre/src/godot/godot/modules/text_server_adv/text_server_adv.h:883:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x45e12da) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #9 Label::_shape() /home/rcorre/src/godot/godot/scene/gui/label.cpp:150:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x6c8c905) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #10 Label::get_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/label.cpp:619:30 (godot.linuxbsd.editor.x86_64.llvm.san+0x6c9366b) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #11 Control::_update_minimum_size_cache() /home/rcorre/src/godot/godot/scene/gui/control.cpp:1608:18 (godot.linuxbsd.editor.x86_64.llvm.san+0x6b69405) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #12 Control::get_combined_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/control.cpp:1627:32 (godot.linuxbsd.editor.x86_64.llvm.san+0x6b69405)
    #13 AcceptDialog::_get_contents_minimum_size() const /home/rcorre/src/godot/godot/scene/gui/dialogs.cpp:260:28 (godot.linuxbsd.editor.x86_64.llvm.san+0x6bb809a) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #14 Window::get_contents_minimum_size() const /home/rcorre/src/godot/godot/scene/main/window.cpp:1566:9 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #15 Window::get_clamped_minimum_size() const /home/rcorre/src/godot/godot/scene/main/window.cpp:1574:22 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c)
    #16 Window::_update_window_size() /home/rcorre/src/godot/godot/scene/main/window.cpp:883:22 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3bf9c)
    #17 Window::set_min_size(Vector2i const&) /home/rcorre/src/godot/godot/scene/main/window.cpp:391:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x6a3cac4) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #18 ConfirmationDialog::ConfirmationDialog() /home/rcorre/src/godot/godot/scene/gui/dialogs.cpp:447:2 (godot.linuxbsd.editor.x86_64.llvm.san+0x6bbcff1) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #19 EditorFileDialog::EditorFileDialog() /home/rcorre/src/godot/godot/editor/editor_file_dialog.cpp:1704:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x5263f09) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #20 EditorAssetLibrary::EditorAssetLibrary(bool) /home/rcorre/src/godot/godot/editor/plugins/asset_library_editor_plugin.cpp:1622:15 (godot.linuxbsd.editor.x86_64.llvm.san+0x5e7a7b9) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #21 ProjectManager::ProjectManager() /home/rcorre/src/godot/godot/editor/project_manager.cpp:2924:19 (godot.linuxbsd.editor.x86_64.llvm.san+0x58d8e94) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #22 Main::start() /home/rcorre/src/godot/godot/main/main.cpp:3002:31 (godot.linuxbsd.editor.x86_64.llvm.san+0x2f2b926) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)
    #23 main /home/rcorre/src/godot/godot/platform/linuxbsd/godot_linuxbsd.cpp:71:6 (godot.linuxbsd.editor.x86_64.llvm.san+0x2e67e41) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb)

SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) (/home/rcorre/src/godot/godot/bin/godot.linuxbsd.editor.x86_64.llvm.san+0x2e12851) (BuildId: fca740c9f8cae87822f94c6eab6045df1899fefb) in pthread_mutex_lock

@rcorre
Copy link
Contributor

rcorre commented Apr 29, 2023

The main pattern I'm seeing is in TextServerAdvanced.

  1. _free_rid takes the THREAD_SAFE_METHOD lock first, then locks FontAdvanced::mutex second
  2. Many functions lock FontAdvanced::mutex first, then call _shaped_text_shape which takes the THREAD_SAFE_METHOD lock second

rcorre added a commit to rcorre/godot that referenced this issue Apr 29, 2023
Fixes godotengine#32081.

_free_rid takes the THREAD_SAFE_METHOD lock first, then locks FontAdvanced::mutex second.
Many functions lock FontAdvanced::mutex first, then call _shaped_text_shape which takes the THREAD_SAFE_METHOD lock second.

TSAN flags this as "lock-order-inversion (potential deadlock)".

As _shaped_text_shape and _free_rid seem to call THREAD_SAFE_METHOD to guard access to the shaped_owner method, I'm assuming that other methods accessing shaped_owner also ought to take a lock. Doing this ensures that the locks are always taken in a consistent order, avoiding a deadlock between _free_rid and the various _shaped_text_* functions.

The full TSAN output can be found at godotengine#32081 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants