Skip to content

Defer storage destruction during Godot->Rust call - #1671

Open
Bromeon wants to merge 4 commits into
masterfrom
bugfix/dead-object-in-calls
Open

Defer storage destruction during Godot->Rust call #1671
Bromeon wants to merge 4 commits into
masterfrom
bugfix/dead-object-in-calls

Conversation

@Bromeon

@Bromeon Bromeon commented Aug 2, 2026

Copy link
Copy Markdown
Member

For RefCounted objects, user code can drop the last ref to itself mid-call (e.g. a signal handler nulling the emitter), which previously freed the storage under the active instance.

I did not go the route of GDScript to add a strong-ref to every Godot->Rust call, for multiple reasons:

  • The problem of rugpulling the own instance still exists for manually managed objects. A panic makes it clear.
  • The extra refcounts cost around ~40% overhead of an empty function call. This is a lot, considering that most Godot->Rust calls never face the rugpulling problem.
  • There are still some edge cases that aren't even covered with the strong-ref approach (e.g. when refcount is already 0 during NOTIFICATION_PREDELETE)

Instead, the storage now counts "claims" on itself (Godot's + 1 per call on the stack), and is freed by whoever releases the last one. This is a bit more complex but retains the benefits of the old version.

Fixes #1666, now covered via itest signal_emitter_destroyed_during_own_call.

@Bromeon Bromeon added bug c: ffi Low-level components and interaction with GDExtension API labels Aug 2, 2026
@Bromeon
Bromeon enabled auto-merge August 2, 2026 13:28
@GodotRust

Copy link
Copy Markdown

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1671

Bromeon added 3 commits August 2, 2026 15:57
For `RefCounted` objects, user code can drop the last ref to itself mid-call
(e.g. a signal handler nulling the emitter), which previously freed the storage
under the active instance.

Now the storage counts "claims" on itself (Godot's + 1 per call on the stack),
and is freed by whoever releases the last one.
The weighed alternatives and cost tables belong in the design notes, not
in rustdoc. Keep the conclusion and the one non-obvious constraint.
The claim keeps the storage alive, but the Godot object itself cannot be
kept: neither free() nor the last reference drop is vetoable. Godot then
dereferences a dangling `this` after the callback returns, e.g. in
Object::_notification_forward().

Reuse the claim counter to detect the situation and report it, instead of
letting it stay silent. Manually managed classes are excluded, since
free() during a call on the object is an established pattern.
@Bromeon
Bromeon force-pushed the bugfix/dead-object-in-calls branch from a3350a9 to 286c85d Compare August 2, 2026 15:29
@Bromeon
Bromeon disabled auto-merge August 2, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug c: ffi Low-level components and interaction with GDExtension API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dropping an RC'd reference within it's own signal causes a hard crash.

2 participants