From 18fbdbb456c07a56b358bea2e392765fbcbb3283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Wed, 26 Feb 2020 11:28:13 +0100 Subject: Reimplement Mutex with C++'s Main: - It's now implemented thanks to ``. No more platform-specific implementations. - `BinaryMutex` (non-recursive) is added, as an alternative for special cases. - Doesn't need allocation/deallocation anymore. It can live in the stack and be part of other classes. - Because of that, it's methods are now `const` and the inner mutex is `mutable` so it can be easily used in `const` contexts. - A no-op implementation is provided if `NO_THREADS` is defined. No more need to add `#ifdef NO_THREADS` just for this. - `MutexLock` now takes a reference. At this point the cases of null `Mutex`es are rare. If you ever need that, just don't use `MutexLock`. - Thread-safe utilities are therefore simpler now. Misc.: - `ScopedMutexLock` is dropped and replaced by `MutexLock`, because they were pretty much the same. - Every case of lock, do-something, unlock is replaced by `MutexLock` (complex cases where it's not straightfoward are kept as as explicit lock and unlock). - `ShaderRD` contained an `std::mutex`, which has been replaced by `Mutex`. --- modules/mono/csharp_script.cpp | 71 +++++++--------------------------- modules/mono/csharp_script.h | 10 ++--- modules/mono/mono_gd/gd_mono_utils.cpp | 4 +- modules/mono/utils/mutex_utils.h | 67 -------------------------------- 4 files changed, 22 insertions(+), 130 deletions(-) delete mode 100644 modules/mono/utils/mutex_utils.h (limited to 'modules/mono') diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index 5c1c098e3e..404d2eb022 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -35,6 +35,7 @@ #include "core/io/json.h" #include "core/os/file_access.h" +#include "core/os/mutex.h" #include "core/os/os.h" #include "core/os/thread.h" #include "core/project_settings.h" @@ -58,7 +59,6 @@ #include "mono_gd/gd_mono_utils.h" #include "signal_awaiter_utils.h" #include "utils/macros.h" -#include "utils/mutex_utils.h" #include "utils/string_utils.h" #include "utils/thread_local.h" @@ -633,7 +633,7 @@ Vector CSharpLanguage::stack_trace_get_info(MonoObjec void CSharpLanguage::post_unsafe_reference(Object *p_obj) { #ifdef DEBUG_ENABLED - SCOPED_MUTEX_LOCK(unsafe_object_references_lock); + MutexLock lock(unsafe_object_references_lock); ObjectID id = p_obj->get_instance_id(); unsafe_object_references[id]++; #endif @@ -641,7 +641,7 @@ void CSharpLanguage::post_unsafe_reference(Object *p_obj) { void CSharpLanguage::pre_unsafe_unreference(Object *p_obj) { #ifdef DEBUG_ENABLED - SCOPED_MUTEX_LOCK(unsafe_object_references_lock); + MutexLock lock(unsafe_object_references_lock); ObjectID id = p_obj->get_instance_id(); Map::Element *elem = unsafe_object_references.find(id); ERR_FAIL_NULL(elem); @@ -764,7 +764,7 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) { List > scripts; { - SCOPED_MUTEX_LOCK(script_instances_mutex); + MutexLock lock(script_instances_mutex); for (SelfList *elem = script_list.first(); elem; elem = elem->next()) { // Cast to CSharpScript to avoid being erased by accident @@ -1204,7 +1204,7 @@ void CSharpLanguage::set_language_index(int p_idx) { void CSharpLanguage::release_script_gchandle(Ref &p_gchandle) { if (!p_gchandle->is_released()) { // Do not lock unnecessarily - SCOPED_MUTEX_LOCK(get_singleton()->script_gchandle_release_mutex); + MutexLock lock(get_singleton()->script_gchandle_release_mutex); p_gchandle->release(); } } @@ -1214,7 +1214,7 @@ void CSharpLanguage::release_script_gchandle(MonoObject *p_expected_obj, Refis_released()) { // Do not lock unnecessarily - SCOPED_MUTEX_LOCK(get_singleton()->script_gchandle_release_mutex); + MutexLock lock(get_singleton()->script_gchandle_release_mutex); MonoObject *target = p_gchandle->get_target(); @@ -1239,24 +1239,6 @@ CSharpLanguage::CSharpLanguage() { gdmono = NULL; -#ifdef NO_THREADS - script_instances_mutex = NULL; - script_gchandle_release_mutex = NULL; - language_bind_mutex = NULL; -#else - script_instances_mutex = Mutex::create(); - script_gchandle_release_mutex = Mutex::create(); - language_bind_mutex = Mutex::create(); -#endif - -#ifdef DEBUG_ENABLED -#ifdef NO_THREADS - unsafe_object_references_lock = NULL; -#else - unsafe_object_references_lock = Mutex::create(); -#endif -#endif - lang_idx = -1; scripts_metadata_invalidated = true; @@ -1269,29 +1251,6 @@ CSharpLanguage::CSharpLanguage() { CSharpLanguage::~CSharpLanguage() { finish(); - - if (script_instances_mutex) { - memdelete(script_instances_mutex); - script_instances_mutex = NULL; - } - - if (language_bind_mutex) { - memdelete(language_bind_mutex); - language_bind_mutex = NULL; - } - - if (script_gchandle_release_mutex) { - memdelete(script_gchandle_release_mutex); - script_gchandle_release_mutex = NULL; - } - -#ifdef DEBUG_ENABLED - if (unsafe_object_references_lock) { - memdelete(unsafe_object_references_lock); - unsafe_object_references_lock = NULL; - } -#endif - singleton = NULL; } @@ -1346,7 +1305,7 @@ bool CSharpLanguage::setup_csharp_script_binding(CSharpScriptBinding &r_script_b void *CSharpLanguage::alloc_instance_binding_data(Object *p_object) { - SCOPED_MUTEX_LOCK(language_bind_mutex); + MutexLock lock(language_bind_mutex); Map::Element *match = script_bindings.find(p_object); if (match) @@ -1381,7 +1340,7 @@ void CSharpLanguage::free_instance_binding_data(void *p_data) { GD_MONO_ASSERT_THREAD_ATTACHED; { - SCOPED_MUTEX_LOCK(language_bind_mutex); + MutexLock lock(language_bind_mutex); Map::Element *data = (Map::Element *)p_data; @@ -2187,7 +2146,7 @@ CSharpInstance::~CSharpInstance() { CSharpScriptBinding &script_binding = ((Map::Element *)data)->get(); if (!script_binding.inited) { - SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->get_language_bind_mutex()); + MutexLock lock(CSharpLanguage::get_singleton()->get_language_bind_mutex()); if (!script_binding.inited) { // Other thread may have set it up // Already had a binding that needs to be setup @@ -2203,7 +2162,7 @@ CSharpInstance::~CSharpInstance() { } if (script.is_valid() && owner) { - SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->script_instances_mutex); + MutexLock lock(CSharpLanguage::get_singleton()->script_instances_mutex); #ifdef DEBUG_ENABLED // CSharpInstance must not be created unless it's going to be added to the list for sure @@ -2979,7 +2938,7 @@ CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_arg instance->_reference_owner_unsafe(); // Here, after assigning the gchandle (for the refcount_incremented callback) { - SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->script_instances_mutex); + MutexLock lock(CSharpLanguage::get_singleton()->script_instances_mutex); instances.insert(instance->owner); } @@ -3067,7 +3026,7 @@ PlaceHolderScriptInstance *CSharpScript::placeholder_instance_create(Object *p_t bool CSharpScript::instance_has(const Object *p_this) const { - SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->script_instances_mutex); + MutexLock lock(CSharpLanguage::get_singleton()->script_instances_mutex); return instances.has((Object *)p_this); } @@ -3140,7 +3099,7 @@ Error CSharpScript::reload(bool p_keep_state) { bool has_instances; { - SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->script_instances_mutex); + MutexLock lock(CSharpLanguage::get_singleton()->script_instances_mutex); has_instances = instances.size(); } @@ -3476,7 +3435,7 @@ CSharpScript::CSharpScript() : #ifdef DEBUG_ENABLED { - SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->script_instances_mutex); + MutexLock lock(CSharpLanguage::get_singleton()->script_instances_mutex); CSharpLanguage::get_singleton()->script_list.add(&this->script_list); } #endif @@ -3485,7 +3444,7 @@ CSharpScript::CSharpScript() : CSharpScript::~CSharpScript() { #ifdef DEBUG_ENABLED - SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->script_instances_mutex); + MutexLock lock(CSharpLanguage::get_singleton()->script_instances_mutex); CSharpLanguage::get_singleton()->script_list.remove(&this->script_list); #endif } diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h index 627218eaf5..f44c4aebc4 100644 --- a/modules/mono/csharp_script.h +++ b/modules/mono/csharp_script.h @@ -325,16 +325,16 @@ class CSharpLanguage : public ScriptLanguage { GDMono *gdmono; SelfList::List script_list; - Mutex *script_instances_mutex; - Mutex *script_gchandle_release_mutex; - Mutex *language_bind_mutex; + Mutex script_instances_mutex; + Mutex script_gchandle_release_mutex; + Mutex language_bind_mutex; Map script_bindings; #ifdef DEBUG_ENABLED // List of unsafe object references Map unsafe_object_references; - Mutex *unsafe_object_references_lock; + Mutex unsafe_object_references_lock; #endif struct StringNameCache { @@ -376,7 +376,7 @@ class CSharpLanguage : public ScriptLanguage { public: StringNameCache string_names; - Mutex *get_language_bind_mutex() { return language_bind_mutex; } + const Mutex &get_language_bind_mutex() { return language_bind_mutex; } _FORCE_INLINE_ int get_language_index() { return lang_idx; } void set_language_index(int p_idx); diff --git a/modules/mono/mono_gd/gd_mono_utils.cpp b/modules/mono/mono_gd/gd_mono_utils.cpp index ae6625a6c6..41f49d8ac9 100644 --- a/modules/mono/mono_gd/gd_mono_utils.cpp +++ b/modules/mono/mono_gd/gd_mono_utils.cpp @@ -33,6 +33,7 @@ #include #include "core/os/dir_access.h" +#include "core/os/mutex.h" #include "core/os/os.h" #include "core/project_settings.h" #include "core/reference.h" @@ -43,7 +44,6 @@ #include "../csharp_script.h" #include "../utils/macros.h" -#include "../utils/mutex_utils.h" #include "gd_mono.h" #include "gd_mono_cache.h" #include "gd_mono_class.h" @@ -74,7 +74,7 @@ MonoObject *unmanaged_get_managed(Object *unmanaged) { CSharpScriptBinding &script_binding = ((Map::Element *)data)->value(); if (!script_binding.inited) { - SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->get_language_bind_mutex()); + MutexLock lock(CSharpLanguage::get_singleton()->get_language_bind_mutex()); if (!script_binding.inited) { // Other thread may have set it up // Already had a binding that needs to be setup diff --git a/modules/mono/utils/mutex_utils.h b/modules/mono/utils/mutex_utils.h deleted file mode 100644 index bafd875395..0000000000 --- a/modules/mono/utils/mutex_utils.h +++ /dev/null @@ -1,67 +0,0 @@ -/*************************************************************************/ -/* mutex_utils.h */ -/*************************************************************************/ -/* This file is part of: */ -/* GODOT ENGINE */ -/* https://godotengine.org */ -/*************************************************************************/ -/* Copyright (c) 2007-2020 Juan Linietsky, Ariel Manzur. */ -/* Copyright (c) 2014-2020 Godot Engine contributors (cf. AUTHORS.md). */ -/* */ -/* Permission is hereby granted, free of charge, to any person obtaining */ -/* a copy of this software and associated documentation files (the */ -/* "Software"), to deal in the Software without restriction, including */ -/* without limitation the rights to use, copy, modify, merge, publish, */ -/* distribute, sublicense, and/or sell copies of the Software, and to */ -/* permit persons to whom the Software is furnished to do so, subject to */ -/* the following conditions: */ -/* */ -/* The above copyright notice and this permission notice shall be */ -/* included in all copies or substantial portions of the Software. */ -/* */ -/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ -/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ -/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/ -/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ -/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ -/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ -/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -/*************************************************************************/ - -#ifndef MUTEX_UTILS_H -#define MUTEX_UTILS_H - -#include "core/error_macros.h" -#include "core/os/mutex.h" - -#include "macros.h" - -class ScopedMutexLock { - Mutex *mutex; - -public: - ScopedMutexLock(Mutex *mutex) { - this->mutex = mutex; -#ifndef NO_THREADS -#ifdef DEBUG_ENABLED - CRASH_COND(!mutex); -#endif - this->mutex->lock(); -#endif - } - - ~ScopedMutexLock() { -#ifndef NO_THREADS -#ifdef DEBUG_ENABLED - CRASH_COND(!mutex); -#endif - mutex->unlock(); -#endif - } -}; - -#define SCOPED_MUTEX_LOCK(m_mutex) ScopedMutexLock GD_UNIQUE_NAME(__scoped_mutex_lock__)(m_mutex); - -// TODO: Add version that receives a lambda instead, once C++11 is allowed - -#endif // MUTEX_UTILS_H -- cgit v1.2.3