Fix crash when creating C# object for native object at the same time on multiple threads

This commit is contained in:
Wojtek Figat
2021-12-16 18:58:14 +01:00
parent 3c3f2ae075
commit 1c34c7f293
4 changed files with 61 additions and 34 deletions

View File

@@ -175,13 +175,16 @@ void Asset::OnDeleteObject()
#endif #endif
} }
void Asset::CreateManaged() bool Asset::CreateManaged()
{ {
// Base // Base
ManagedScriptingObject::CreateManaged(); if (ManagedScriptingObject::CreateManaged())
return true;
// Managed objects holds a reference to this asset until it will be removed by GC // Managed objects holds a reference to this asset until it will be removed by GC
AddReference(); AddReference();
return false;
} }
void Asset::DestroyManaged() void Asset::DestroyManaged()

View File

@@ -241,7 +241,7 @@ public:
// [ManagedScriptingObject] // [ManagedScriptingObject]
String ToString() const override; String ToString() const override;
void OnDeleteObject() override; void OnDeleteObject() override;
void CreateManaged() override; bool CreateManaged() override;
void DestroyManaged() override; void DestroyManaged() override;
void OnManagedInstanceDeleted() override; void OnManagedInstanceDeleted() override;
void OnScriptingDispose() override; void OnScriptingDispose() override;

View File

@@ -59,7 +59,8 @@ ScriptingObject::~ScriptingObject()
MObject* ScriptingObject::GetManagedInstance() const MObject* ScriptingObject::GetManagedInstance() const
{ {
#if USE_MONO #if USE_MONO
return _gcHandle ? mono_gchandle_get_target(_gcHandle) : nullptr; const int32 handle = Platform::AtomicRead((int32*)&_gcHandle);
return handle ? mono_gchandle_get_target(handle) : nullptr;
#else #else
return nullptr; return nullptr;
#endif #endif
@@ -226,10 +227,6 @@ void ScriptingObject::OnScriptingDispose()
MonoObject* ScriptingObject::CreateManagedInternal() MonoObject* ScriptingObject::CreateManagedInternal()
{ {
#if BUILD_DEBUG
ASSERT(!HasManagedInstance());
#endif
// Get class // Get class
MClass* monoClass = GetClass(); MClass* monoClass = GetClass();
if (monoClass == nullptr) if (monoClass == nullptr)
@@ -302,7 +299,6 @@ void ScriptingObject::DestroyManaged()
void ScriptingObject::RegisterObject() void ScriptingObject::RegisterObject()
{ {
ASSERT(!IsRegistered()); ASSERT(!IsRegistered());
Flags |= ObjectFlags::IsRegistered; Flags |= ObjectFlags::IsRegistered;
Scripting::RegisterObject(this); Scripting::RegisterObject(this);
} }
@@ -310,7 +306,6 @@ void ScriptingObject::RegisterObject()
void ScriptingObject::UnregisterObject() void ScriptingObject::UnregisterObject()
{ {
ASSERT(IsRegistered()); ASSERT(IsRegistered());
Flags &= ~ObjectFlags::IsRegistered; Flags &= ~ObjectFlags::IsRegistered;
Scripting::UnregisterObject(this); Scripting::UnregisterObject(this);
} }
@@ -378,24 +373,39 @@ ManagedScriptingObject::ManagedScriptingObject(const SpawnParams& params)
{ {
} }
void ManagedScriptingObject::CreateManaged() bool ManagedScriptingObject::CreateManaged()
{ {
#if USE_MONO #if USE_MONO
MonoObject* managedInstance = CreateManagedInternal(); MonoObject* managedInstance = CreateManagedInternal();
if (managedInstance) if (!managedInstance)
{ return true;
// Cache the GC handle to the object (used to track the target object because it can be moved in a memory) // Cache the GC handle to the object (used to track the target object because it can be moved in a memory)
_gcHandle = mono_gchandle_new_weakref(managedInstance, false); auto handle = mono_gchandle_new_weakref(managedInstance, false);
auto oldHandle = Platform::InterlockedCompareExchange((int32*)&_gcHandle, *(int32*)&handle, 0);
if (*(uint32*)&oldHandle != 0)
{
// Other thread already created the object before
if (const auto monoClass = GetClass())
{
// Reset managed to unmanaged pointer
const MField* monoUnmanagedPtrField = monoClass->GetField(ScriptingObject_unmanagedPtr);
if (monoUnmanagedPtrField)
{
void* param = nullptr;
monoUnmanagedPtrField->SetValue(managedInstance, &param);
}
}
mono_gchandle_free(handle);
return true;
}
#endif
// Ensure to be registered // Ensure to be registered
if (!IsRegistered()) if (!IsRegistered())
RegisterObject(); RegisterObject();
}
#else return false;
// Ensure to be registered
if (!IsRegistered())
RegisterObject();
#endif
} }
PersistentScriptingObject::PersistentScriptingObject(const SpawnParams& params) PersistentScriptingObject::PersistentScriptingObject(const SpawnParams& params)
@@ -432,30 +442,44 @@ void PersistentScriptingObject::OnScriptingDispose()
// Don't delete C++ object // Don't delete C++ object
} }
void PersistentScriptingObject::CreateManaged() bool PersistentScriptingObject::CreateManaged()
{ {
#if USE_MONO #if USE_MONO
MonoObject* managedInstance = CreateManagedInternal(); MonoObject* managedInstance = CreateManagedInternal();
if (managedInstance) if (!managedInstance)
return true;
// Prevent form object GC destruction
auto handle = mono_gchandle_new(managedInstance, false);
auto oldHandle = Platform::InterlockedCompareExchange((int32*)&_gcHandle, *(int32*)&handle, 0);
if (*(uint32*)&oldHandle != 0)
{ {
// Prevent form object GC destruction and moving // Other thread already created the object before
_gcHandle = mono_gchandle_new(managedInstance, false); if (const auto monoClass = GetClass())
{
// Reset managed to unmanaged pointer
const MField* monoUnmanagedPtrField = monoClass->GetField(ScriptingObject_unmanagedPtr);
if (monoUnmanagedPtrField)
{
void* param = nullptr;
monoUnmanagedPtrField->SetValue(managedInstance, &param);
}
}
mono_gchandle_free(handle);
return true;
}
#endif
// Ensure to be registered // Ensure to be registered
if (!IsRegistered()) if (!IsRegistered())
RegisterObject(); RegisterObject();
}
#else return false;
// Ensure to be registered
if (!IsRegistered())
RegisterObject();
#endif
} }
class ScriptingObjectInternal class ScriptingObjectInternal
{ {
public: public:
#if !COMPILE_WITHOUT_CSHARP #if !COMPILE_WITHOUT_CSHARP
static MonoObject* Create1(MonoReflectionType* type) static MonoObject* Create1(MonoReflectionType* type)

View File

@@ -182,7 +182,7 @@ public:
virtual void OnManagedInstanceDeleted(); virtual void OnManagedInstanceDeleted();
virtual void OnScriptingDispose(); virtual void OnScriptingDispose();
virtual void CreateManaged() = 0; virtual bool CreateManaged() = 0;
virtual void DestroyManaged(); virtual void DestroyManaged();
public: public:
@@ -239,7 +239,7 @@ public:
public: public:
// [ScriptingObject] // [ScriptingObject]
void CreateManaged() override; bool CreateManaged() override;
}; };
/// <summary> /// <summary>
@@ -266,5 +266,5 @@ public:
// [ManagedScriptingObject] // [ManagedScriptingObject]
void OnManagedInstanceDeleted() override; void OnManagedInstanceDeleted() override;
void OnScriptingDispose() override; void OnScriptingDispose() override;
void CreateManaged() override; bool CreateManaged() override;
}; };