maxmustermann/godot
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fix Nativescript thread safety

Browse source
This commit is contained in:
Manuele Finocchiaro 2021-10-26 19:39:41 +01:00
parent 5d0ec1779d
commit 3da9e15223
6 changed files with 87 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
core/object.cpp
View file

@ -1984,7 +1984,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]);
}
}
}

2
core/script_language.h
View file

@ -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

111
modules/gdnative/nativescript/nativescript.cpp
View file

@ -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<Vector<void *> *>::Element *E = binding_instances.front(); E; E = E->next()) {
Vector<void *> &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<void *, Pair<RWLock, Vector<void *>> *>::Element *E = binding_instances.front(); E; E = E->next()) {
Vector<void *> &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<void *> *binding_data = (Vector<void *> *)p_object->get_script_instance_binding(lang_idx);
Pair<RWLock, Vector<void *>> *binding_data = (Pair<RWLock, Vector<void *>> *)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<void *> *binding_data = new Vector<void *>;
{
// 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<RWLock, Vector<void *>> *binding_data = new Pair<RWLock, Vector<void *>>;
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<void *> &binding_data = *(Vector<void *> *)p_data;
Pair<RWLock, Vector<void *>> &binding_data = (*(Pair<RWLock, Vector<void *>> *)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<void *> &binding_data = *(Vector<void *> *)data;
Vector<void *> &binding_data = (*(Pair<RWLock, Vector<void *>> *)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<void *> &binding_data = *(Vector<void *> *)data;
Vector<void *> &binding_data = (*(Pair<RWLock, Vector<void *>> *)data).second;
bool can_die = true;

6
modules/gdnative/nativescript/nativescript.h
View file

@ -251,7 +251,9 @@ private:
void call_libraries_cb(const StringName &name);
Vector<Pair<bool, godot_instance_binding_functions>> binding_functions;
Set<Vector<void *> *> binding_instances;
RWLock binding_instances_lock;
Map<void *, Pair<RWLock, Vector<void *>> *> binding_instances;
Map<int, HashMap<StringName, const void *>> 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);

2
modules/mono/csharp_script.cpp
View file

@ -1308,7 +1308,7 @@ Map<Object *, CSharpScriptBinding>::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());

2
modules/mono/csharp_script.h
View file

@ -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);