You've already forked godot
mirror of
https://github.com/godotengine/godot.git
synced 2025-11-08 12:40:44 +00:00
Merge pull request #83670 from raulsntos/notification-predelete-cleanup
Add `NOTIFICATION_PREDELETE_CLEANUP` notification to fix C# `Dispose()`
This commit is contained in:
@@ -198,6 +198,7 @@ bool Object::_predelete() {
|
|||||||
notification(NOTIFICATION_PREDELETE, true);
|
notification(NOTIFICATION_PREDELETE, true);
|
||||||
if (_predelete_ok) {
|
if (_predelete_ok) {
|
||||||
_class_name_ptr = nullptr; // Must restore, so constructors/destructors have proper class name access at each stage.
|
_class_name_ptr = nullptr; // Must restore, so constructors/destructors have proper class name access at each stage.
|
||||||
|
notification(NOTIFICATION_PREDELETE_CLEANUP, true);
|
||||||
}
|
}
|
||||||
return _predelete_ok;
|
return _predelete_ok;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -801,6 +801,8 @@ public:
|
|||||||
NOTIFICATION_POSTINITIALIZE = 0,
|
NOTIFICATION_POSTINITIALIZE = 0,
|
||||||
NOTIFICATION_PREDELETE = 1,
|
NOTIFICATION_PREDELETE = 1,
|
||||||
NOTIFICATION_EXTENSION_RELOADED = 2,
|
NOTIFICATION_EXTENSION_RELOADED = 2,
|
||||||
|
// Internal notification to send after NOTIFICATION_PREDELETE, not bound to scripting.
|
||||||
|
NOTIFICATION_PREDELETE_CLEANUP = 3,
|
||||||
};
|
};
|
||||||
|
|
||||||
/* TYPE API */
|
/* TYPE API */
|
||||||
|
|||||||
@@ -1989,24 +1989,31 @@ const Variant CSharpInstance::get_rpc_config() const {
|
|||||||
|
|
||||||
void CSharpInstance::notification(int p_notification, bool p_reversed) {
|
void CSharpInstance::notification(int p_notification, bool p_reversed) {
|
||||||
if (p_notification == Object::NOTIFICATION_PREDELETE) {
|
if (p_notification == Object::NOTIFICATION_PREDELETE) {
|
||||||
// When NOTIFICATION_PREDELETE is sent, we also take the chance to call Dispose().
|
if (base_ref_counted) {
|
||||||
// It's safe to call Dispose() multiple times and NOTIFICATION_PREDELETE is guaranteed
|
// At this point, Dispose() was already called (manually or from the finalizer).
|
||||||
|
// The RefCounted wouldn't have reached 0 otherwise, since the managed side
|
||||||
|
// references it and Dispose() needs to be called to release it.
|
||||||
|
// However, this means C# RefCounted scripts can't receive NOTIFICATION_PREDELETE, but
|
||||||
|
// this is likely the case with GDScript as well: https://github.com/godotengine/godot/issues/6784
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
} else if (p_notification == Object::NOTIFICATION_PREDELETE_CLEANUP) {
|
||||||
|
// When NOTIFICATION_PREDELETE_CLEANUP is sent, we also take the chance to call Dispose().
|
||||||
|
// It's safe to call Dispose() multiple times and NOTIFICATION_PREDELETE_CLEANUP is guaranteed
|
||||||
// to be sent at least once, which happens right before the call to the destructor.
|
// to be sent at least once, which happens right before the call to the destructor.
|
||||||
|
|
||||||
predelete_notified = true;
|
predelete_notified = true;
|
||||||
|
|
||||||
if (base_ref_counted) {
|
if (base_ref_counted) {
|
||||||
// It's not safe to proceed if the owner derives RefCounted and the refcount reached 0.
|
// At this point, Dispose() was already called (manually or from the finalizer).
|
||||||
// At this point, Dispose() was already called (manually or from the finalizer) so
|
// The RefCounted wouldn't have reached 0 otherwise, since the managed side
|
||||||
// that's not a problem. The refcount wouldn't have reached 0 otherwise, since the
|
// references it and Dispose() needs to be called to release it.
|
||||||
// managed side references it and Dispose() needs to be called to release it.
|
|
||||||
// However, this means C# RefCounted scripts can't receive NOTIFICATION_PREDELETE, but
|
|
||||||
// this is likely the case with GDScript as well: https://github.com/godotengine/godot/issues/6784
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
_call_notification(p_notification, p_reversed);
|
// NOTIFICATION_PREDELETE_CLEANUP is not sent to scripts.
|
||||||
|
// After calling Dispose() the C# instance can no longer be used,
|
||||||
|
// so it should be the last thing we do.
|
||||||
GDMonoCache::managed_callbacks.CSharpInstanceBridge_CallDispose(
|
GDMonoCache::managed_callbacks.CSharpInstanceBridge_CallDispose(
|
||||||
gchandle.get_intptr(), /* okIfNull */ false);
|
gchandle.get_intptr(), /* okIfNull */ false);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user