Test RID_Owner thread safety

SConstruct

@@ -850,6 +850,7 @@ if env.msvc and not methods.using_clang(env):  # MSVC
                 "/wd4245",
                 "/wd4267",
                 "/wd4305",  # C4305 (truncation): double to float or real_t, too hard to avoid.
+                "/wd4324",  # C4820 (structure was padded due to alignment specifier)
                 "/wd4514",  # C4514 (unreferenced inline function has been removed)
                 "/wd4714",  # C4714 (function marked as __forceinline not inlined)
                 "/wd4820",  # C4820 (padding added after construct)

core/config/project_settings.cpp

@@ -1485,7 +1485,7 @@ ProjectSettings::ProjectSettings() {
 	GLOBAL_DEF("display/window/subwindows/embed_subwindows", true);
 	// Keep the enum values in sync with the `DisplayServer::VSyncMode` enum.
 	custom_prop_info["display/window/vsync/vsync_mode"] = PropertyInfo(Variant::INT, "display/window/vsync/vsync_mode", PROPERTY_HINT_ENUM, "Disabled,Enabled,Adaptive,Mailbox");
-	custom_prop_info["rendering/driver/threads/thread_model"] = PropertyInfo(Variant::INT, "rendering/driver/threads/thread_model", PROPERTY_HINT_ENUM, "Single-Unsafe,Single-Safe,Multi-Threaded");
+	GLOBAL_DEF("rendering/driver/run_on_separate_thread", false);
 	GLOBAL_DEF("physics/2d/run_on_separate_thread", false);
 	GLOBAL_DEF("physics/3d/run_on_separate_thread", false);
 

core/io/resource_loader.cpp

@@ -872,7 +872,7 @@ bool ResourceLoader::_ensure_load_progress() {
 	// Some servers may need a new engine iteration to allow the load to progress.
 	// Since the only known one is the rendering server (in single thread mode), let's keep it simple and just sync it.
 	// This may be refactored in the future to support other servers and have less coupling.
-	if (OS::get_singleton()->get_render_thread_mode() == OS::RENDER_SEPARATE_THREAD) {
+	if (OS::get_singleton()->is_separate_thread_rendering_enabled()) {
 		return false; // Not needed.
 	}
 	RenderingServer::get_singleton()->sync();

core/os/os.h

@@ -92,19 +92,13 @@ class OS {
 	typedef void (*ImeCallback)(void *p_inp, const String &p_text, Point2 p_selection);
 	typedef bool (*HasServerFeatureCallback)(const String &p_feature);
 
-	enum RenderThreadMode {
-		RENDER_THREAD_UNSAFE,
-		RENDER_THREAD_SAFE,
-		RENDER_SEPARATE_THREAD
-	};
-
 protected:
 	friend class Main;
 	// Needed by tests to setup command-line args.
 	friend int test_main(int argc, char *argv[]);
 
 	HasServerFeatureCallback has_server_feature_callback = nullptr;
-	RenderThreadMode _render_thread_mode = RENDER_THREAD_SAFE;
+	bool _separate_thread_render = false;
 
 	// Functions used by Main to initialize/deinitialize the OS.
 	void add_logger(Logger *p_logger);
@@ -262,7 +256,7 @@ class OS {
 	virtual uint64_t get_static_memory_peak_usage() const;
 	virtual Dictionary get_memory_info() const;
 
-	RenderThreadMode get_render_thread_mode() const { return _render_thread_mode; }
+	bool is_separate_thread_rendering_enabled() const { return _separate_thread_render; }
 
 	virtual String get_locale() const;
 	String get_locale_language() const;

core/os/spin_lock.h

@@ -31,43 +31,72 @@
 #ifndef SPIN_LOCK_H
 #define SPIN_LOCK_H
 
-#include "core/typedefs.h"
+#include "core/os/thread.h"
 
-#if defined(__APPLE__)
+#include <atomic>
+#include <thread>
 
-#include <os/lock.h>
+static_assert(std::atomic_bool::is_always_lock_free);
 
-class SpinLock {
-	mutable os_unfair_lock _lock = OS_UNFAIR_LOCK_INIT;
+class alignas(Thread::CACHE_LINE_BYTES) AcqRelSpinLock {
+	mutable std::atomic<bool> locked = ATOMIC_VAR_INIT(false);
 
 public:
 	_ALWAYS_INLINE_ void lock() const {
-		os_unfair_lock_lock(&_lock);
+		while (true) {
+			bool expected = false;
+			if (locked.compare_exchange_weak(expected, true, std::memory_order_acquire, std::memory_order_relaxed)) {
+				break;
+			}
+			do {
+				std::this_thread::yield();
+			} while (locked.load(std::memory_order_relaxed));
+		}
 	}
 
 	_ALWAYS_INLINE_ void unlock() const {
-		os_unfair_lock_unlock(&_lock);
+		locked.store(false, std::memory_order_release);
+	}
+
+	_ALWAYS_INLINE_ void acquire() const {
+		(void)locked.load(std::memory_order_acquire);
+	}
+
+	_ALWAYS_INLINE_ void release() const {
+		// Do as little as possible to issue a release on the atomic
+		// without changing its value.
+		while (true) {
+			for (int i = 0; i < 2; i++) {
+				bool expected = (bool)i;
+				if (locked.compare_exchange_weak(expected, expected, std::memory_order_release, std::memory_order_relaxed)) {
+					return;
+				}
+			}
+		}
 	}
 };
 
-#else
+#if defined(__APPLE__)
 
-#include <atomic>
+#include <os/lock.h>
 
 class SpinLock {
-	mutable std::atomic_flag locked = ATOMIC_FLAG_INIT;
+	mutable os_unfair_lock _lock = OS_UNFAIR_LOCK_INIT;
 
 public:
 	_ALWAYS_INLINE_ void lock() const {
-		while (locked.test_and_set(std::memory_order_acquire)) {
-			// Continue.
-		}
+		os_unfair_lock_lock(&_lock);
 	}
+
 	_ALWAYS_INLINE_ void unlock() const {
-		locked.clear(std::memory_order_release);
+		os_unfair_lock_unlock(&_lock);
 	}
 };
 
+#else
+
+using SpinLock = AcqRelSpinLock;
+
 #endif // __APPLE__
 
 #endif // SPIN_LOCK_H

core/os/thread.h

@@ -42,6 +42,8 @@
 #include "core/templates/safe_refcount.h"
 #include "core/typedefs.h"
 
+#include <new>
+
 #ifdef MINGW_ENABLED
 #define MINGW_STDTHREAD_REDUNDANCY_WARNING
 #include "thirdparty/mingw-std-threads/mingw.thread.h"
@@ -85,6 +87,19 @@ class Thread {
 		void (*term)() = nullptr;
 	};
 
+#if defined(__cpp_lib_hardware_interference_size) && !defined(ANDROID_ENABLED) // This would be OK with NDK >= 26.
+#if defined(__GNUC__) && !defined(__clang__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Winterference-size"
+#endif
+	static const size_t CACHE_LINE_BYTES = std::hardware_destructive_interference_size;
+#if defined(__GNUC__) && !defined(__clang__)
+#pragma GCC diagnostic pop
+#endif
+#else
+	static const size_t CACHE_LINE_BYTES = 64;
+#endif
+
 private:
 	friend class Main;
 
@@ -116,6 +131,8 @@ class Thread {
 
 	_FORCE_INLINE_ static bool is_main_thread() { return caller_id == MAIN_ID; } // Gain a tiny bit of perf here because there is no need to validate caller_id here, because only main thread will be set as 1.
 
+	_FORCE_INLINE_ static void yield() { std::this_thread::yield(); }
+
 	static Error set_name(const String &p_name);
 
 	ID start(Thread::Callback p_callback, void *p_user, const Settings &p_settings = Settings());
@@ -135,6 +152,8 @@ class Thread {
 
 	typedef uint64_t ID;
 
+	static const size_t CACHE_LINE_BYTES = sizeof(void *);
+
 	enum : ID {
 		UNASSIGNED_ID = 0,
 		MAIN_ID = 1
@@ -176,6 +195,8 @@ class Thread {
 
 	_FORCE_INLINE_ static bool is_main_thread() { return true; }
 
+	_FORCE_INLINE_ static void yield() {}
+
 	static Error set_name(const String &p_name) { return ERR_UNAVAILABLE; }
 
 	void start(Thread::Callback p_callback, void *p_user, const Settings &p_settings = Settings()) {}