Skip to content

Conversation

@Faless
Copy link
Contributor

@Faless Faless commented Jul 29, 2022

This is an attempt to make the lifecycle of wrapped objects clearer.

Godot keeps track of bindings' userdata for each object it creates.

This allows allocating the memory of the wrapper only once per object even if that object is passed multiple times between binding code and godot code.

The binding information is composed of multiple functions, this includes a callback for when the userdata is to be allocated (called once) and for when the userdata is to be deallocated (again, called once).

When allocating data with "memnew" we set the object bindings during the postinitialize phase, but surely we shouldn't do that when allocating the userdata as a result of bindings callback themselves.

Additionally, since we let Godot handle (and track) raw memory allocation and de-allocation, we need to manually call the deconstructor of the wrapper class during the free callback, to ensure that its non-trivial members are correctly de-initialized.

Fixes #679

This is an attempt to make the lifecycle of wrapped objects clearer.
Godot keeps track of bindings' userdata for each object it creates.
This allows allocating the memory of the wrapper only once per object
even if that object is passed multiple times between binding code and
godot code.

The binding information is composed of multiple functions, this includes
a callback for when the userdata is to be allocated (called once) and
for when the userdata is to be deallocated (again, called once).

When allocating data with "memnew" we set the object bindings during the
postinitialize phase, but surely we shouldn't do that when allocating
the userdata as a result of bindings callback themselves.

Additionally, since we let Godot handle (and track) raw memory
allocation and de-allocation, we need to manually call the deconstructor
of the wrapper class during the free callback, to ensure that its
non-trivial members are correctly de-initialized.
@Faless Faless added bug This has been identified as a bug topic:gdextension This relates to the new Godot 4 extension implementation labels Jul 29, 2022
@Faless Faless added this to the 4.0 milestone Jul 29, 2022
@akien-mga akien-mga requested a review from a team July 29, 2022 06:37
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@akien-mga akien-mga merged commit 54e1385 into godotengine:master Jul 29, 2022
@akien-mga
Copy link
Member

Thanks!

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

Labels

bug This has been identified as a bug topic:gdextension This relates to the new Godot 4 extension implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_instance_bindings is initialized multiple times

3 participants