godotengine/godot
1
0
Fork 0
mirror of https://github.com/godotengine/godot.git synced 2025-12-31 18:41:20 +00:00
Code Releases Wiki Activity

Avoid deadlocks in multi-threaded management of the C# script map

Browse Source
This commit is contained in:
Pedro J. Estébanez
2024-11-29 10:10:49 +01:00
parent 9e6098432a
commit f8f5505b26
2 changed files with 110 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

165
modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs
View File

@@ -433,16 +433,29 @@ namespace Godot.Bridge
private static unsafe bool AddScriptBridgeCore(IntPtr scriptPtr, string scriptPath)
{
lock (_scriptTypeBiMap.ReadWriteLock)
_scriptTypeBiMap.ReadWriteLock.EnterUpgradeableReadLock();
try
{
if (!_scriptTypeBiMap.IsScriptRegistered(scriptPtr))
{
if (!_pathTypeBiMap.TryGetScriptType(scriptPath, out Type? scriptType))
return false;
_scriptTypeBiMap.Add(scriptPtr, scriptType);
_scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
try
{
_scriptTypeBiMap.Add(scriptPtr, scriptType);
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
}
}
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitUpgradeableReadLock();
}
return true;
}
@@ -465,7 +478,8 @@ namespace Godot.Bridge
private static unsafe void GetOrCreateScriptBridgeForType(Type scriptType, godot_ref* outScript)
{
lock (_scriptTypeBiMap.ReadWriteLock)
_scriptTypeBiMap.ReadWriteLock.EnterUpgradeableReadLock();
try
{
if (_scriptTypeBiMap.TryGetScriptPtr(scriptType, out IntPtr scriptPtr))
{
@@ -477,6 +491,10 @@ namespace Godot.Bridge
// This path is slower, but it's only executed for the first instantiation of the type
CreateScriptBridgeForType(scriptType, outScript);
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitUpgradeableReadLock();
}
}
internal static unsafe void GetOrLoadOrCreateScriptForType(Type scriptType, godot_ref* outScript)
@@ -484,7 +502,8 @@ namespace Godot.Bridge
static bool GetPathOtherwiseGetOrCreateScript(Type scriptType, godot_ref* outScript,
[MaybeNullWhen(false)] out string scriptPath)
{
lock (_scriptTypeBiMap.ReadWriteLock)
_scriptTypeBiMap.ReadWriteLock.EnterUpgradeableReadLock();
try
{
if (_scriptTypeBiMap.TryGetScriptPtr(scriptType, out IntPtr scriptPtr))
{
@@ -512,6 +531,10 @@ namespace Godot.Bridge
scriptPath = null;
return false;
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitUpgradeableReadLock();
}
}
static string GetVirtualConstructedGenericTypeScriptPath(Type scriptType, string scriptPath)
@@ -541,7 +564,16 @@ namespace Godot.Bridge
// IMPORTANT: The virtual path must be added to _pathTypeBiMap before the first
// load of the script, otherwise the loaded script won't be added to _scriptTypeBiMap.
scriptPath = GetVirtualConstructedGenericTypeScriptPath(scriptType, scriptPath);
_pathTypeBiMap.Add(scriptPath, scriptType);
_scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
try
{
_pathTypeBiMap.Add(scriptPath, scriptType);
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
}
}
// This must be done outside the read-write lock, as the script resource loading can lock it
@@ -571,89 +603,108 @@ namespace Godot.Bridge
{
Debug.Assert(!scriptType.IsGenericTypeDefinition, $"Script type must be a constructed generic type or not generic at all. Type: {scriptType}.");
NativeFuncs.godotsharp_internal_new_csharp_script(outScript);
IntPtr scriptPtr = outScript->Reference;
_scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
try
{
NativeFuncs.godotsharp_internal_new_csharp_script(outScript);
IntPtr scriptPtr = outScript->Reference;
// Caller takes care of locking
_scriptTypeBiMap.Add(scriptPtr, scriptType);
_scriptTypeBiMap.Add(scriptPtr, scriptType);
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
}
NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
NativeFuncs.godotsharp_internal_reload_registered_script(outScript->Reference);
}
[UnmanagedCallersOnly]
internal static void RemoveScriptBridge(IntPtr scriptPtr)
{
_scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
try
{
lock (_scriptTypeBiMap.ReadWriteLock)
{
_scriptTypeBiMap.Remove(scriptPtr);
}
_scriptTypeBiMap.Remove(scriptPtr);
}
catch (Exception e)
{
ExceptionUtils.LogException(e);
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
}
}
[UnmanagedCallersOnly]
internal static godot_bool TryReloadRegisteredScriptWithClass(IntPtr scriptPtr)
{
_scriptTypeBiMap.ReadWriteLock.EnterUpgradeableReadLock();
try
{
lock (_scriptTypeBiMap.ReadWriteLock)
if (_scriptTypeBiMap.TryGetScriptType(scriptPtr, out _))
{
if (_scriptTypeBiMap.TryGetScriptType(scriptPtr, out _))
{
// NOTE:
// Currently, we reload all scripts, not only the ones from the unloaded ALC.
// As such, we need to handle this case instead of treating it as an error.
NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
return godot_bool.True;
}
if (!_scriptDataForReload.TryGetValue(scriptPtr, out var dataForReload))
{
GD.PushError("Missing class qualified name for reloading script");
return godot_bool.False;
}
_ = _scriptDataForReload.TryRemove(scriptPtr, out _);
if (dataForReload.assemblyName == null)
{
GD.PushError(
$"Missing assembly name of class '{dataForReload.classFullName}' for reloading script");
return godot_bool.False;
}
var scriptType = ReflectionUtils.FindTypeInLoadedAssemblies(dataForReload.assemblyName,
dataForReload.classFullName);
if (scriptType == null)
{
// The class was removed, can't reload
return godot_bool.False;
}
if (!typeof(GodotObject).IsAssignableFrom(scriptType))
{
// The class no longer inherits GodotObject, can't reload
return godot_bool.False;
}
_scriptTypeBiMap.Add(scriptPtr, scriptType);
// NOTE:
// Currently, we reload all scripts, not only the ones from the unloaded ALC.
// As such, we need to handle this case instead of treating it as an error.
NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
return godot_bool.True;
}
if (!_scriptDataForReload.TryGetValue(scriptPtr, out var dataForReload))
{
GD.PushError("Missing class qualified name for reloading script");
return godot_bool.False;
}
_ = _scriptDataForReload.TryRemove(scriptPtr, out _);
if (dataForReload.assemblyName == null)
{
GD.PushError(
$"Missing assembly name of class '{dataForReload.classFullName}' for reloading script");
return godot_bool.False;
}
var scriptType = ReflectionUtils.FindTypeInLoadedAssemblies(dataForReload.assemblyName,
dataForReload.classFullName);
if (scriptType == null)
{
// The class was removed, can't reload
return godot_bool.False;
}
if (!typeof(GodotObject).IsAssignableFrom(scriptType))
{
// The class no longer inherits GodotObject, can't reload
return godot_bool.False;
}
_scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
try
{
_scriptTypeBiMap.Add(scriptPtr, scriptType);
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
}
NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
return godot_bool.True;
}
catch (Exception e)
{
ExceptionUtils.LogException(e);
return godot_bool.False;
}
finally
{
_scriptTypeBiMap.ReadWriteLock.ExitUpgradeableReadLock();
}
}
private static unsafe void GetScriptTypeInfo(Type scriptType, godot_csharp_type_info* outTypeInfo)

3
modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.types.cs
View File

@@ -4,6 +4,7 @@ using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
namespace Godot.Bridge;
@@ -13,7 +14,7 @@ public static partial class ScriptManagerBridge
{
private class ScriptTypeBiMap
{
public readonly object ReadWriteLock = new();
public readonly ReaderWriterLockSlim ReadWriteLock = new(LockRecursionPolicy.SupportsRecursion);
private System.Collections.Generic.Dictionary<IntPtr, Type> _scriptTypeMap = new();
private System.Collections.Generic.Dictionary<Type, IntPtr> _typeScriptMap = new();