godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #75533 from RedworkDE/net-no-reload-noncollectible 

C#: Fix editor integration breaking and causing error spam when reloading assemblies fails
This commit is contained in:
Yuri Sizov 2023-06-22 12:45:02 +02:00 committed by GitHub
parent b0299c9ad1 e0f644a48d
commit 57e61db115
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 142 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
doc/classes/ProjectSettings.xml
View File

@ -818,6 +818,9 @@
<member name="dotnet/project/assembly_name" type="String" setter="" getter="" default="&quot;&quot;">
Name of the .NET assembly. This name is used as the name of the [code].csproj[/code] and [code].sln[/code] files. By default, it's set to the name of the project ([member application/config/name]) allowing to change it in the future without affecting the .NET assembly.
</member>
<member name="dotnet/project/assembly_reload_attempts" type="int" setter="" getter="" default="3">
Number of times to attempt assembly reloading after rebuilding .NET assembies. Effectively also the timeout in seconds to wait for unloading of script assemblies to finish.
</member>
<member name="dotnet/project/solution_directory" type="String" setter="" getter="" default="&quot;&quot;">
Directory that contains the [code].sln[/code] file. By default, the [code].sln[/code] files is in the root of the project directory, next to the [code]project.godot[/code] and [code].csproj[/code] files.
Changing this value allows setting up a multi-project scenario where there are multiple [code].csproj[/code]. Keep in mind that the Godot project is considered one of the C# projects in the workspace and it's root directory should contain the [code]project.godot[/code] and [code].csproj[/code] next to each other.

1
main/main.cpp
View File

@ -2756,6 +2756,7 @@ bool Main::start() {
// Default values should be synced with mono_gd/gd_mono.cpp.
GLOBAL_DEF("dotnet/project/assembly_name", "");
GLOBAL_DEF("dotnet/project/solution_directory", "");
GLOBAL_DEF(PropertyInfo(Variant::INT, "dotnet/project/assembly_reload_attempts", PROPERTY_HINT_RANGE, "1,16,1,or_greater"), 3);
#endif
Error err;

46
modules/mono/csharp_script.cpp
View File

@ -120,6 +120,7 @@ void CSharpLanguage::init() {
GLOBAL_DEF("dotnet/project/assembly_name", "");
#ifdef TOOLS_ENABLED
GLOBAL_DEF("dotnet/project/solution_directory", "");
GLOBAL_DEF(PropertyInfo(Variant::INT, "dotnet/project/assembly_reload_attempts", PROPERTY_HINT_RANGE, "1,16,1,or_greater"), 3);
#endif
gdmono = memnew(GDMono);
@ -770,10 +771,6 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
return;
}
// TODO:
// Currently, this reloads all scripts, including those whose class is not part of the
// assembly load context being unloaded. As such, we unnecessarily reload GodotTools.
print_verbose(".NET: Reloading assemblies...");
// There is no soft reloading with Mono. It's always hard reloading.
@ -784,8 +781,19 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
MutexLock lock(script_instances_mutex);
for (SelfList<CSharpScript> *elem = script_list.first(); elem; elem = elem->next()) {
// Cast to CSharpScript to avoid being erased by accident
scripts.push_back(Ref<CSharpScript>(elem->self()));
bool is_reloadable = false;
for (Object *obj : elem->self()->instances) {
ERR_CONTINUE(!obj->get_script_instance());
CSharpInstance *csi = static_cast<CSharpInstance *>(obj->get_script_instance());
if (GDMonoCache::managed_callbacks.GCHandleBridge_GCHandleIsTargetCollectible(csi->get_gchandle_intptr())) {
is_reloadable = true;
break;
}
}
if (is_reloadable) {
// Cast to CSharpScript to avoid being erased by accident.
scripts.push_back(Ref<CSharpScript>(elem->self()));
}
}
}
@ -800,6 +808,10 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
ERR_CONTINUE(managed_callable->delegate_handle.value == nullptr);
if (!GDMonoCache::managed_callbacks.GCHandleBridge_GCHandleIsTargetCollectible(managed_callable->delegate_handle)) {
continue;
}
Array serialized_data;
bool success = GDMonoCache::managed_callbacks.DelegateUtils_TrySerializeDelegateWithGCHandle(
@ -907,6 +919,15 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
scr->_clear();
}
// Release the delegates that were serialized earlier.
{
MutexLock lock(ManagedCallable::instances_mutex);
for (KeyValue<ManagedCallable *, Array> &kv : ManagedCallable::instances_pending_reload) {
kv.key->release_delegate_handle();
}
}
// Do domain reload
if (gdmono->reload_project_assemblies() != OK) {
// Failed to reload the scripts domain
@ -1158,19 +1179,6 @@ bool CSharpLanguage::debug_break(const String &p_error, bool p_allow_continue) {
}
}
void CSharpLanguage::_on_scripts_domain_about_to_unload() {
#ifdef GD_MONO_HOT_RELOAD
{
MutexLock lock(ManagedCallable::instances_mutex);
for (SelfList<ManagedCallable> *elem = ManagedCallable::instances.first(); elem; elem = elem->next()) {
ManagedCallable *managed_callable = elem->self();
managed_callable->release_delegate_handle();
}
}
#endif
}
#ifdef TOOLS_ENABLED
void CSharpLanguage::_editor_init_callback() {
// Load GodotTools and initialize GodotSharpEditor

1
modules/mono/csharp_script.h
View File

@ -347,7 +347,6 @@ class CSharpLanguage : public ScriptLanguage {
String _debug_error;
friend class GDMono;
void _on_scripts_domain_about_to_unload();
#ifdef TOOLS_ENABLED
EditorPlugin *godotsharp_editor = nullptr;

44
modules/mono/glue/GodotSharp/GodotPlugins/Main.cs
View File

@ -21,6 +21,13 @@ namespace GodotPlugins
private sealed class PluginLoadContextWrapper
{
private PluginLoadContext? _pluginLoadContext;
private readonly WeakReference _weakReference;
private PluginLoadContextWrapper(PluginLoadContext pluginLoadContext, WeakReference weakReference)
{
_pluginLoadContext = pluginLoadContext;
_weakReference = weakReference;
}
public string? AssemblyLoadedPath
{
@ -31,7 +38,14 @@ namespace GodotPlugins
public bool IsCollectible
{
[MethodImpl(MethodImplOptions.NoInlining)]
get => _pluginLoadContext?.IsCollectible ?? false;
// if _pluginLoadContext is null we already started unloading, so it was collectible
get => _pluginLoadContext?.IsCollectible ?? true;
}
public bool IsAlive
{
[MethodImpl(MethodImplOptions.NoInlining)]
get => _weakReference.IsAlive;
}
[MethodImpl(MethodImplOptions.NoInlining)]
@ -43,19 +57,13 @@ namespace GodotPlugins
bool isCollectible
)
{
var wrapper = new PluginLoadContextWrapper();
wrapper._pluginLoadContext = new PluginLoadContext(
pluginPath, sharedAssemblies, mainLoadContext, isCollectible);
var assembly = wrapper._pluginLoadContext.LoadFromAssemblyName(assemblyName);
var context = new PluginLoadContext(pluginPath, sharedAssemblies, mainLoadContext, isCollectible);
var reference = new WeakReference(context, trackResurrection: true);
var wrapper = new PluginLoadContextWrapper(context, reference);
var assembly = context.LoadFromAssemblyName(assemblyName);
return (assembly, wrapper);
}
[MethodImpl(MethodImplOptions.NoInlining)]
public WeakReference CreateWeakReference()
{
return new WeakReference(_pluginLoadContext, trackResurrection: true);
}
[MethodImpl(MethodImplOptions.NoInlining)]
internal void Unload()
{
@ -165,7 +173,7 @@ namespace GodotPlugins
if (_editorApiAssembly == null)
throw new InvalidOperationException("The Godot editor API assembly is not loaded.");
var (assembly, _) = LoadPlugin(assemblyPath, isCollectible: _editorHint);
var (assembly, _) = LoadPlugin(assemblyPath, isCollectible: false);
NativeLibrary.SetDllImportResolver(assembly, _dllImportResolver!);
@ -236,32 +244,29 @@ namespace GodotPlugins
Console.WriteLine("Unloading assembly load context...");
var alcWeakReference = pluginLoadContext.CreateWeakReference();
pluginLoadContext.Unload();
pluginLoadContext = null;
int startTimeMs = Environment.TickCount;
bool takingTooLong = false;
while (alcWeakReference.IsAlive)
while (pluginLoadContext.IsAlive)
{
GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced);
GC.WaitForPendingFinalizers();
if (!alcWeakReference.IsAlive)
if (!pluginLoadContext.IsAlive)
break;
int elapsedTimeMs = Environment.TickCount - startTimeMs;
if (!takingTooLong && elapsedTimeMs >= 2000)
if (!takingTooLong && elapsedTimeMs >= 200)
{
takingTooLong = true;
// TODO: How to log from GodotPlugins? (delegate pointer?)
Console.Error.WriteLine("Assembly unloading is taking longer than expected...");
}
else if (elapsedTimeMs >= 5000)
else if (elapsedTimeMs >= 1000)
{
// TODO: How to log from GodotPlugins? (delegate pointer?)
Console.Error.WriteLine(
@ -273,6 +278,7 @@ namespace GodotPlugins
Console.WriteLine("Assembly load context unloaded successfully.");
pluginLoadContext = null;
return true;
}
catch (Exception e)

21
modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/GCHandleBridge.cs
View File

@ -18,5 +18,26 @@ namespace Godot.Bridge
ExceptionUtils.LogException(e);
}
}
// Returns true, if releasing the provided handle is necessary for assembly unloading to succeed.
// This check is not perfect and only intended to prevent things in GodotTools from being reloaded.
[UnmanagedCallersOnly]
internal static godot_bool GCHandleIsTargetCollectible(IntPtr gcHandlePtr)
{
try
{
var target = GCHandle.FromIntPtr(gcHandlePtr).Target;
if (target is Delegate @delegate)
return DelegateUtils.IsDelegateCollectible(@delegate).ToGodotBool();
return target.GetType().IsCollectible.ToGodotBool();
}
catch (Exception e)
{
ExceptionUtils.LogException(e);
return godot_bool.True;
}
}
}
}

2
modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ManagedCallbacks.cs
View File

@ -38,6 +38,7 @@ namespace Godot.Bridge
public delegate* unmanaged<IntPtr, godot_dictionary*, godot_dictionary*, void> CSharpInstanceBridge_SerializeState;
public delegate* unmanaged<IntPtr, godot_dictionary*, godot_dictionary*, void> CSharpInstanceBridge_DeserializeState;
public delegate* unmanaged<IntPtr, void> GCHandleBridge_FreeGCHandle;
public delegate* unmanaged<IntPtr, godot_bool> GCHandleBridge_GCHandleIsTargetCollectible;
public delegate* unmanaged<void*, void> DebuggingUtils_GetCurrentStackInfo;
public delegate* unmanaged<void> DisposablesTracker_OnGodotShuttingDown;
public delegate* unmanaged<godot_bool, void> GD_OnCoreApiAssemblyLoaded;
@ -78,6 +79,7 @@ namespace Godot.Bridge
CSharpInstanceBridge_SerializeState = &CSharpInstanceBridge.SerializeState,
CSharpInstanceBridge_DeserializeState = &CSharpInstanceBridge.DeserializeState,
GCHandleBridge_FreeGCHandle = &GCHandleBridge.FreeGCHandle,
GCHandleBridge_GCHandleIsTargetCollectible = &GCHandleBridge.GCHandleIsTargetCollectible,
DebuggingUtils_GetCurrentStackInfo = &DebuggingUtils.GetCurrentStackInfo,
DisposablesTracker_OnGodotShuttingDown = &DisposablesTracker.OnGodotShuttingDown,
GD_OnCoreApiAssemblyLoaded = &GD.OnCoreApiAssemblyLoaded,

32
modules/mono/glue/GodotSharp/GodotSharp/Core/DelegateUtils.cs
View File

@ -560,6 +560,38 @@ namespace Godot
return type;
}
// Returns true, if unloading the delegate is necessary for assembly unloading to succeed.
// This check is not perfect and only intended to prevent things in GodotTools from being reloaded.
internal static bool IsDelegateCollectible(Delegate @delegate)
{
if (@delegate.GetType().IsCollectible)
return true;
if (@delegate is MulticastDelegate multicastDelegate)
{
Delegate[] invocationList = multicastDelegate.GetInvocationList();
if (invocationList.Length > 1)
{
foreach (Delegate oneDelegate in invocationList)
if (IsDelegateCollectible(oneDelegate))
return true;
return false;
}
}
if (@delegate.Method.IsCollectible)
return true;
object? target = @delegate.Target;
if (target is not null && target.GetType().IsCollectible)
return true;
return false;
}
internal static class RuntimeTypeConversionHelper
{
[SuppressMessage("ReSharper", "RedundantNameQualifier")]

30
modules/mono/mono_gd/gd_mono.cpp
View File

@ -488,15 +488,31 @@ bool GDMono::_load_project_assembly() {
#endif
#ifdef GD_MONO_HOT_RELOAD
void GDMono::reload_failure() {
if (++project_load_failure_count >= (int)GLOBAL_GET("dotnet/project/assembly_reload_attempts")) {
// After reloading a project has failed n times in a row, update the path and modification time
// to stop any further attempts at loading this assembly, which probably is never going to work anyways.
project_load_failure_count = 0;
ERR_PRINT_ED(".NET: Giving up on assembly reloading. Please restart the editor if unloading was failing.");
String assembly_name = path::get_csharp_project_name();
String assembly_path = GodotSharpDirs::get_res_temp_assemblies_dir().path_join(assembly_name + ".dll");
assembly_path = ProjectSettings::get_singleton()->globalize_path(assembly_path);
project_assembly_path = assembly_path.simplify_path();
project_assembly_modified_time = FileAccess::get_modified_time(assembly_path);
}
}
Error GDMono::reload_project_assemblies() {
ERR_FAIL_COND_V(!runtime_initialized, ERR_BUG);
finalizing_scripts_domain = true;
CSharpLanguage::get_singleton()->_on_scripts_domain_about_to_unload();
if (!get_plugin_callbacks().UnloadProjectPluginCallback()) {
ERR_FAIL_V_MSG(Error::FAILED, ".NET: Failed to unload assemblies.");
ERR_PRINT_ED(".NET: Failed to unload assemblies. Please check https://github.com/godotengine/godot/issues/78513 for more information.");
reload_failure();
return FAILED;
}
finalizing_scripts_domain = false;
@ -504,10 +520,16 @@ Error GDMono::reload_project_assemblies() {
// Load the project's main assembly. Here, during hot-reloading, we do
// consider failing to load the project's main assembly to be an error.
if (!_load_project_assembly()) {
print_error(".NET: Failed to load project assembly.");
ERR_PRINT_ED(".NET: Failed to load project assembly.");
reload_failure();
return ERR_CANT_OPEN;
}
if (project_load_failure_count > 0) {
project_load_failure_count = 0;
ERR_PRINT_ED(".NET: Assembly reloading succeeded after failures.");
}
return OK;
}
#endif

2
modules/mono/mono_gd/gd_mono.h
View File

@ -68,6 +68,7 @@ class GDMono {
String project_assembly_path;
uint64_t project_assembly_modified_time = 0;
int project_load_failure_count = 0;
#ifdef TOOLS_ENABLED
bool _load_project_assembly();
@ -144,6 +145,7 @@ public:
#endif
#ifdef GD_MONO_HOT_RELOAD
void reload_failure();
Error reload_project_assemblies();
#endif

1
modules/mono/mono_gd/gd_mono_cache.cpp
View File

@ -79,6 +79,7 @@ void update_godot_api_cache(const ManagedCallbacks &p_managed_callbacks) {
CHECK_CALLBACK_NOT_NULL(CSharpInstanceBridge, SerializeState);
CHECK_CALLBACK_NOT_NULL(CSharpInstanceBridge, DeserializeState);
CHECK_CALLBACK_NOT_NULL(GCHandleBridge, FreeGCHandle);
CHECK_CALLBACK_NOT_NULL(GCHandleBridge, GCHandleIsTargetCollectible);
CHECK_CALLBACK_NOT_NULL(DebuggingUtils, GetCurrentStackInfo);
CHECK_CALLBACK_NOT_NULL(DisposablesTracker, OnGodotShuttingDown);
CHECK_CALLBACK_NOT_NULL(GD, OnCoreApiAssemblyLoaded);

2
modules/mono/mono_gd/gd_mono_cache.h
View File

@ -104,6 +104,7 @@ struct ManagedCallbacks {
using FuncCSharpInstanceBridge_SerializeState = void(GD_CLR_STDCALL *)(GCHandleIntPtr, const Dictionary *, const Dictionary *);
using FuncCSharpInstanceBridge_DeserializeState = void(GD_CLR_STDCALL *)(GCHandleIntPtr, const Dictionary *, const Dictionary *);
using FuncGCHandleBridge_FreeGCHandle = void(GD_CLR_STDCALL *)(GCHandleIntPtr);
using FuncGCHandleBridge_GCHandleIsTargetCollectible = bool(GD_CLR_STDCALL *)(GCHandleIntPtr);
using FuncDebuggingUtils_GetCurrentStackInfo = void(GD_CLR_STDCALL *)(Vector<ScriptLanguage::StackInfo> *);
using FuncDisposablesTracker_OnGodotShuttingDown = void(GD_CLR_STDCALL *)();
using FuncGD_OnCoreApiAssemblyLoaded = void(GD_CLR_STDCALL *)(bool);
@ -138,6 +139,7 @@ struct ManagedCallbacks {
FuncCSharpInstanceBridge_SerializeState CSharpInstanceBridge_SerializeState;
FuncCSharpInstanceBridge_DeserializeState CSharpInstanceBridge_DeserializeState;
FuncGCHandleBridge_FreeGCHandle GCHandleBridge_FreeGCHandle;
FuncGCHandleBridge_GCHandleIsTargetCollectible GCHandleBridge_GCHandleIsTargetCollectible;
FuncDebuggingUtils_GetCurrentStackInfo DebuggingUtils_GetCurrentStackInfo;
FuncDisposablesTracker_OnGodotShuttingDown DisposablesTracker_OnGodotShuttingDown;
FuncGD_OnCoreApiAssemblyLoaded GD_OnCoreApiAssemblyLoaded;