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

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

Open
Mercerenies opened this issue Jan 24, 2023 · 5 comments
Milestone

Comments

@Mercerenies
Copy link

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
Copy link
Contributor

Can you reproduce it in Godot 4?

@Mercerenies
Copy link
Author

@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
Copy link
Contributor

Godot v4.0.beta1.official

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

@Mercerenies
Copy link
Author

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

@lawnjelly
Copy link
Member

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

@lawnjelly lawnjelly modified the milestones: 3.7, 4.x Oct 28, 2024
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