Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C# Adding more than one delegate to the same signal makes disconnect behave irratically #77354

Closed
Zoomulator opened this issue May 22, 2023 · 4 comments

Comments

@Zoomulator
Copy link

Zoomulator commented May 22, 2023

Godot version

v4.0.3.stable.mono.official [5222a99]

System information

Windows 10

Issue description

Disconnecting a delegate (Action) from a signal who has been connected to more than once will sometimes (non-deterministically) cause a "nonexistent signal" error:

E 0:00:00:0911   Godot.NativeInterop.NativeFuncs.generated.cs:332 @ void Godot.NativeInterop.NativeFuncs.godotsharp_method_bind_ptrcall(IntPtr , IntPtr , System.Void** , System.Void* ): Disconnecting nonexistent signal 'tree_exiting', callable: Delegate::Invoke.
  <C++ Error>    Condition "!s->slot_map.has(*p_callable.get_base_comparator())" is true.
  <C++ Source>   core/object/object.cpp:1331 @ _disconnect()
  <Stack Trace>  Godot.NativeInterop.NativeFuncs.generated.cs:332 @ void Godot.NativeInterop.NativeFuncs.godotsharp_method_bind_ptrcall(IntPtr , IntPtr , System.Void** , System.Void* )
                 NativeCalls.cs:3541 @ void Godot.NativeCalls.godot_icall_2_412(IntPtr , IntPtr , Godot.NativeInterop.godot_string_name , Godot.Callable& )
                 GodotObject.cs:784 @ void Godot.GodotObject.Disconnect(Godot.StringName , Godot.Callable )
                 Node.cs:1646 @ void Godot.Node.remove_TreeExiting(System.Action )
                 Main.cs:20 @ void Main.TestOperator()
                 Main.cs:8 @ void Main._Ready()
                 Node.cs:1813 @ Boolean Godot.Node.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name& , Godot.NativeInterop.NativeVariantPtrArgs , Godot.NativeInterop.godot_variant& )
                 Main_ScriptMethods.generated.cs:45 @ Boolean Main.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name& , Godot.NativeInterop.NativeVariantPtrArgs , Godot.NativeInterop.godot_variant& )
                 CSharpInstanceBridge.cs:24 @ Godot.NativeInterop.godot_bool Godot.Bridge.CSharpInstanceBridge.Call(IntPtr , Godot.NativeInterop.godot_string_name* , Godot.NativeInterop.godot_variant** , Int32 , Godot.NativeInterop.godot_variant_call_error* , Godot.NativeInterop.godot_variant* )

Steps to reproduce

In a C# script for a Node, create two delegates, connect both to the same signal and then disconnect them.
This will sometimes give the error.

var c0 = () => GD.Print("c0");
var c1 = () => GD.Print("c1");

TreeExiting += c0;
TreeExiting += c1;
TreeExiting -= c0; // Sometimes ERROR
TreeExiting -= c1;

Minimal reproduction project

SignalDelegateDisconnect.zip

The scripts repeats 20 times to assure that the error is encountered. If there is no error, try it a few more times.

@Zoomulator
Copy link
Author

Addendum: Actually it seems that it's the same for methods used as handlers as well, not just delegates.

@RedworkDE
Copy link
Member

Confirmed on 4.0.3, no longer an issue on master (v4.1.dev.mono.custom_build 5dc093b)

(Almost certainly the MRP for the last point of #76708 (comment))

@Zoomulator
Copy link
Author

Zoomulator commented May 23, 2023

I built the latest master and can confirm that it's fixed by #77199

You guys work so fast I second guess causality itself. Thanks!

@Zoomulator
Copy link
Author

I cherry-picked some commits to get it working on my local build based on 4.0.3. Maybe possible to include these in the next maintenance release?

git cherry-pick -m 1 e0e93ce094d19d132f34aa5c92e892524a8125c9
git cherry-pick 1a4eccf7e74027c6204eed2fc06fe4309cdd8a2b
git cherry-pick 1cfc382fe89ba9df3d9edc2de07fbc28e594193a

from these pull requests
#72421
#76728
#77199

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants