diff --git a/core/object.cpp b/core/object.cpp index e81524bd9b..797fd92324 100644 --- a/core/object.cpp +++ b/core/object.cpp @@ -1985,7 +1985,7 @@ Object::~Object() { if (!ScriptServer::are_languages_finished()) { for (int i = 0; i < MAX_SCRIPT_INSTANCE_BINDINGS; i++) { if (_script_instance_bindings[i]) { - ScriptServer::get_language(i)->free_instance_binding_data(_script_instance_bindings[i]); + ScriptServer::get_language(i)->free_instance_binding_data(this, _script_instance_bindings[i]); } } } diff --git a/core/script_language.h b/core/script_language.h index f276ff670f..c622d6761e 100644 --- a/core/script_language.h +++ b/core/script_language.h @@ -359,7 +359,7 @@ public: virtual int profiling_get_frame_data(ProfilingInfo *p_info_arr, int p_info_max) = 0; virtual void *alloc_instance_binding_data(Object *p_object) { return nullptr; } //optional, not used by all languages - virtual void free_instance_binding_data(void *p_data) {} //optional, not used by all languages + virtual void free_instance_binding_data(Object *p_object, void *p_data) {} //optional, not used by all languages virtual void refcount_incremented_instance_binding(Object *p_object) {} //optional, not used by all languages virtual bool refcount_decremented_instance_binding(Object *p_object) { return true; } //return true if it can die //optional, not used by all languages diff --git a/modules/gdnative/nativescript/nativescript.cpp b/modules/gdnative/nativescript/nativescript.cpp index c74200da97..023ad0b56b 100644 --- a/modules/gdnative/nativescript/nativescript.cpp +++ b/modules/gdnative/nativescript/nativescript.cpp @@ -30,6 +30,8 @@ #include "nativescript.h" +#include "core/os/mutex.h" +#include "core/os/rw_lock.h" #include "gdnative/gdnative.h" #include "core/core_string_names.h" @@ -1307,8 +1309,10 @@ int NativeScriptLanguage::register_binding_functions(godot_instance_binding_func void NativeScriptLanguage::unregister_binding_functions(int p_idx) { ERR_FAIL_INDEX(p_idx, binding_functions.size()); - for (Set *>::Element *E = binding_instances.front(); E; E = E->next()) { - Vector &binding_data = *E->get(); + // This should only be entered by the main thread after all objectts have been collected, so I assume + // this doesn't need to be thread safe + for (Map> *>::Element *E = binding_instances.front(); E; E = E->next()) { + Vector &binding_data = E->get()->second; if (p_idx < binding_data.size() && binding_data[p_idx] && binding_functions[p_idx].second.free_instance_binding_data) { binding_functions[p_idx].second.free_instance_binding_data(binding_functions[p_idx].second.data, binding_data[p_idx]); @@ -1327,65 +1331,108 @@ void *NativeScriptLanguage::get_instance_binding_data(int p_idx, Object *p_objec ERR_FAIL_COND_V_MSG(!binding_functions[p_idx].first, nullptr, "Tried to get binding data for a nativescript binding that does not exist."); - Vector *binding_data = (Vector *)p_object->get_script_instance_binding(lang_idx); + Pair> *binding_data = (Pair> *)p_object->get_script_instance_binding(lang_idx); if (!binding_data) { return nullptr; // should never happen. } - if (binding_data->size() <= p_idx) { - // okay, add new elements here. - int old_size = binding_data->size(); - - binding_data->resize(p_idx + 1); - - for (int i = old_size; i <= p_idx; i++) { - (*binding_data).write[i] = NULL; + // We can get here on muliple threads at the same time. + // Check lucky scenario first with shared access. + { + RWLockRead rlock(binding_data->first); + if (binding_data->second.size() > p_idx && binding_data->second[p_idx]) { + return binding_data->second[p_idx]; } } - if (!(*binding_data)[p_idx]) { - const void *global_type_tag = get_global_type_tag(p_idx, p_object->get_class_name()); + // Gotta work to do with exclusive access. + { + RWLockWrite wlock(binding_data->first); - // no binding data yet, soooooo alloc new one \o/ - (*binding_data).write[p_idx] = binding_functions[p_idx].second.alloc_instance_binding_data(binding_functions[p_idx].second.data, global_type_tag, (godot_object *)p_object); + // Need to check again, this might have already been allocated + if (binding_data->second.size() > p_idx && binding_data->second[p_idx]) { + return binding_data->second[p_idx]; + } + + if (binding_data->second.size() <= p_idx) { + // okay, add new elements here. + int old_size = binding_data->second.size(); + + binding_data->second.resize(p_idx + 1); + + for (int i = old_size; i <= p_idx; i++) { + binding_data->second.write[i] = NULL; + } + } + + if (!binding_data->second[p_idx]) { + const void *global_type_tag = get_global_type_tag(p_idx, p_object->get_class_name()); + + // no binding data yet, soooooo alloc new one \o/ + binding_data->second.write[p_idx] = binding_functions[p_idx].second.alloc_instance_binding_data(binding_functions[p_idx].second.data, global_type_tag, (godot_object *)p_object); + } + + return binding_data->second[p_idx]; } - - return (*binding_data)[p_idx]; } void *NativeScriptLanguage::alloc_instance_binding_data(Object *p_object) { - Vector *binding_data = new Vector; + { + // Look at the binding instances with shared access. + // If data is there return it + RWLockRead rlock(binding_instances_lock); - binding_data->resize(binding_functions.size()); - - for (int i = 0; i < binding_functions.size(); i++) { - (*binding_data).write[i] = NULL; + if (binding_instances.has(p_object)) { + return binding_instances[p_object]; + } } - binding_instances.insert(binding_data); + { + // The data was not there, lock with exclusive access. + RWLockWrite wlock(binding_instances_lock); - return (void *)binding_data; + // We need to check again as that might have changed from the shared access check. + if (binding_instances.has(p_object)) { + return binding_instances[p_object]; + } + + // Data is not there, create it. + Pair> *binding_data = new Pair>; + + binding_data->second.resize(binding_functions.size()); + + for (int i = 0; i < binding_functions.size(); i++) { + binding_data->second.write[i] = NULL; + } + + binding_instances.insert(p_object, binding_data); + + return (void *)binding_data; + } } -void NativeScriptLanguage::free_instance_binding_data(void *p_data) { +void NativeScriptLanguage::free_instance_binding_data(Object *p_object, void *p_data) { if (!p_data) { return; } - Vector &binding_data = *(Vector *)p_data; + Pair> &binding_data = (*(Pair> *)p_data); - for (int i = 0; i < binding_data.size(); i++) { - if (!binding_data[i]) { + for (int i = 0; i < binding_data.second.size(); i++) { + if (!binding_data.second[i]) { continue; } if (binding_functions[i].first && binding_functions[i].second.free_instance_binding_data) { - binding_functions[i].second.free_instance_binding_data(binding_functions[i].second.data, binding_data[i]); + binding_functions[i].second.free_instance_binding_data(binding_functions[i].second.data, binding_data.second[i]); } } - binding_instances.erase(&binding_data); + { + RWLockWrite wlock(binding_instances_lock); + binding_instances.erase(p_object); + } delete &binding_data; } @@ -1397,7 +1444,7 @@ void NativeScriptLanguage::refcount_incremented_instance_binding(Object *p_objec return; } - Vector &binding_data = *(Vector *)data; + Vector &binding_data = (*(Pair> *)data).second; for (int i = 0; i < binding_data.size(); i++) { if (!binding_data[i]) { @@ -1421,7 +1468,7 @@ bool NativeScriptLanguage::refcount_decremented_instance_binding(Object *p_objec return true; } - Vector &binding_data = *(Vector *)data; + Vector &binding_data = (*(Pair> *)data).second; bool can_die = true; diff --git a/modules/gdnative/nativescript/nativescript.h b/modules/gdnative/nativescript/nativescript.h index a28a383887..d7a8f26e9a 100644 --- a/modules/gdnative/nativescript/nativescript.h +++ b/modules/gdnative/nativescript/nativescript.h @@ -251,7 +251,9 @@ private: void call_libraries_cb(const StringName &name); Vector> binding_functions; - Set *> binding_instances; + + RWLock binding_instances_lock; + Map> *> binding_instances; Map> global_type_tags; @@ -350,7 +352,7 @@ public: void *get_instance_binding_data(int p_idx, Object *p_object); virtual void *alloc_instance_binding_data(Object *p_object); - virtual void free_instance_binding_data(void *p_data); + virtual void free_instance_binding_data(Object *p_object, void *p_data); virtual void refcount_incremented_instance_binding(Object *p_object); virtual bool refcount_decremented_instance_binding(Object *p_object); diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index 8a5be78102..0c18afce55 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -1308,7 +1308,7 @@ Map::Element *CSharpLanguage::insert_script_bindi return script_bindings.insert(p_object, p_script_binding); } -void CSharpLanguage::free_instance_binding_data(void *p_data) { +void CSharpLanguage::free_instance_binding_data(Object *p_object, void *p_data) { if (GDMono::get_singleton() == NULL) { #ifdef DEBUG_ENABLED CRASH_COND(!script_bindings.empty()); diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h index 1b568c4828..af858df768 100644 --- a/modules/mono/csharp_script.h +++ b/modules/mono/csharp_script.h @@ -460,7 +460,7 @@ public: // Don't use these. I'm watching you virtual void *alloc_instance_binding_data(Object *p_object); - virtual void free_instance_binding_data(void *p_data); + virtual void free_instance_binding_data(Object *p_object, void *p_data); virtual void refcount_incremented_instance_binding(Object *p_object); virtual bool refcount_decremented_instance_binding(Object *p_object);