One-shot signal fails to disconnect if the reference is freed during signal resolution

[Mercerenies]

Godot version

v3.5.stable.official [991bb6a]

System information

Fedora 31

Issue description

Consider the following code.

extends Node2D

class A:
    signal signal_a

class B:
    signal signal_b
    var a

    func _init(a_):
        a = a_
        a.connect("signal_a", self, "_on_signal_a", [], CONNECT_ONESHOT)

    func _on_signal_a():
        emit_signal("signal_b")

func example_runner(b):
    yield(b, "signal_b")
    print("End of example_runner")

func _ready():
    var a = A.new()
    example_runner(B.new(a))
    yield(get_tree(), "idle_frame")
    a.emit_signal("signal_a")

A is a simple object that emits a signal. B contains an A and forwards the signal_a into its own signal_b. Crucially, it does this only once (CONNECT_ONESHOT is set on the signal connection). Both are reference types, not nodes.

Then we create an A and a B. We keep a reference to the A in _ready (basically, for the duration of this test, but the only living reference to the B is in example_runner. example_runner yields to signal_b and then prints a simple message to the screen. This is good enough to keep a reference to our B object alive, at least until signal_b fires.

In _ready, we wait a frame and then emit signal_a. This causes our B object (which is being kept alive by the example_runner function state) to receive signal_a and emit its own signal_b. In turn, this causes example_runner to finish executing (printing its string to the console) and terminate.

Expected behavior: I expect all of this to happen with no errors.

Actual behavior: The code still executes as expected, but the terminal shows an error that

0:00:00.483 _disconnect: Disconnecting nonexistent signal 'signal_a', slot: 0:_on_signal_a.

I suspect that the sequence of events is as follows.

1. Line 25 runs. signal_a is emitted.
2. The B object (which currently has one reference to it) gets signal_a. This calls _on_signal_a.
3. _on_signal_a emits signal_b.
4. example_runner was waiting on signal_b, so it resumes. It prints something and finishes executing.
5. example_runner held the only reference to our B. So the B object is freed at this time, by reference counting semantics.
6. The C++ code that disconnects one-shot signals runs, attempting to disconnect our signal. But the B object it was set on is already freed. Error!

The C++ code in Object::emit_signal is keeping a reference to our target object B, but it seems to not update the reference count, which allows the B object to be freed while C++ still holds a reference to it and intends to do something with it.

Steps to reproduce

The above code snippet is available in an attached project below. Simply run the Main.tscn scene. You will see an error appear in the console, as indicated above.

Minimal reproduction project

Workspace - Referenced Oneshot Signal.zip

[YuriSizov]

Can you reproduce it in Godot 4?

[Mercerenies]

@YuriSizov Yes, I have just tested this on Godot v4.0.beta1.official [20d6672] and the problem is reproducible there as well (after replacing idle_frame with process_frame). Error message:

E 0:00:00:0425   _ready: Cannot disconnect 'signal_a' from callable 'null::_on_signal_a': the callable object is null.
  <C++ Error>    Condition "!target_object" is true.
  <C++ Source>   core/object/object.cpp:1320 @ _disconnect()
  <Stack Trace>  Main.gd:25 @ _ready()

[YuriSizov]

Godot v4.0.beta1.official

That's several months old, can you test with RC2 instead?

[Mercerenies]

I can also reproduce it successfully on the linked release candidate.

[lawnjelly]

Closed by #97464 in 3.x, reassigning to 4.x (although it may already be closed there, needs verifying).

[akien-mga]

Converted the MRP to 4.x syntax, there doesn't seem to be a bug in 4.5-beta1:

extends Node2D

class A:
    signal signal_a

class B:
    signal signal_b
    var a

    func _init(a_):
        a = a_
        a.signal_a.connect(_on_signal_a, CONNECT_ONE_SHOT)

    func _on_signal_a():
        signal_b.emit()

func example_runner(b):
    await b.signal_b
    print("End of example_runner")

func _ready():
    var a = A.new()
    example_runner(B.new(a))
    await get_tree().process_frame
    a.signal_a.emit()

It's reproducible in 4.0 and 4.1 with this error:

ERROR: Cannot disconnect 'signal_a' from callable 'null::_on_signal_a': the callable object is null.
   at: _disconnect (core/object/object.cpp:1321)

And in 4.2 with this:

ERROR: Attempt to disconnect a nonexistent connection from '<RefCounted#-9223372009205922620>'. Signal: 'signal_a', callable: 'null::_on_signal_a'.
   at: _disconnect (core/object/object.cpp:1420)

But it seems fixed in 4.3 and later.