Skip to content

Fix _THREAD_SAFE_CLASS_ and related macros#2026

Open
dsnopek wants to merge 1 commit into
godotengine:masterfrom
dsnopek:fix-mutex-lock
Open

Fix _THREAD_SAFE_CLASS_ and related macros#2026
dsnopek wants to merge 1 commit into
godotengine:masterfrom
dsnopek:fix-mutex-lock

Conversation

@dsnopek

@dsnopek dsnopek commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Fixes #2020

This also adds some tests, so hopefully, we won't break it again later

@dsnopek dsnopek added this to the 10.x milestone Jul 25, 2026
@dsnopek
dsnopek requested a review from a team as a code owner July 25, 2026 11:44
@dsnopek dsnopek added the bug This has been identified as a bug label Jul 25, 2026
namespace godot {

class MutexLock {
const Mutex &mutex;

@Ivorforce Ivorforce Jul 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fix the issue I think, but...

I think we should sync Mutex over directly instead and push Mutex into a CoreBind namespace.
The CoreBind Mutex only really exists for scripting languages that can't do better. I can't think of a reason to use the CoreBind one instead of the template one; it's not allowed to be allocated locally (which was violated here), and it's way slower too.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's an option!

I had thought we were using a lot of OS-specific code in Mutex but reading it now, it's mostly std::mutex (except on mingw) so syncing should be fine

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I submitted #2032 for consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This has been identified as a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Macros "_THREAD_SAFE_XXX" in " include/godot_cpp/core/mutex_lock.hpp" are not work correctly in godot-cpp.

2 participants