Potential Memory Leak because of move_and_slide() against Each Collidable Bodies

[Lazy-Rabbit-2001]

Godot version

4.2 beta 3 ~ 4.3 dev 6

System information

Windows 11, GLES3

Issue description

This is an extended issue from #57073 opened by @BeayemX.
At first I didn't realize that get_slide_collision() could be a memory leaker until I found something abnormal yesterday when I test a GDExtension, in which I used this call to get the last slide collision and emit some signals with this object, but when the debug began and I tested it for seconds, I found the "Object" amount in profiler was increasing. And then I searched around the issues tab and finally found that issue at the beginning of this report.
But things is always coming with dark clouds. When I returned to remove this call, a weird thing happened: the amount of object was still increasing after the removal of using the method! To research more, I started figuring out what happened with this phenomenon and eventually found this dark cloud:
I opened a new project with only a CharacterBody2D using template moving code, and a TileMaps with tiles. Every time the move_and_slide() is called and the collision between the body calling the method and another one that the body haven't collided with (if it's a TileMap, each piece of tile is considered as an individual collision body), the amount of collision increased.

Could this be a high-priority problem?

Yes, and the reason lays on two aspects:

1. Most of developers are not aware of this issue, which made them call this method more, increasing the risk to lead to memory leak
2. Quoted from the issue in the first line:

Looking to backtraces on gdb, ret_opaque is a pointer to a Ref that references count increases in method->ptrcall(base_obj, argptrs, ret_opaque); but isn't unreferenced therefore. The references count also increases in VariantInternal::object_assign(ret, *ret_opaque);, but is unferenced at the end of the gdscript scope:

If this is true, then this is a disastrous potential glitch with the process of RefCounted.

Why should this be a independent and new issue rather than response in that issue?

This has been not only an issue related to get_slide_collision(), but even one to move_and_slide() or even to RefCounted.

Steps to reproduce

Make a CharacterBody2D move with move_and_slide() and collide with any collision bodies.

Minimal reproduction project

Issue.zip
(No GDExtensions)

[akien-mga]

Please provide a reproduction project. The exact setup you're using is necessary to reproduce the issue. It sounds like you're using GDExtension which isn't mentioned at all in the steps to reproduce, and is far from trivial to setup.

[Torguen]

I can't see the video, nothing is shown even if I press the play button.

[Lazy-Rabbit-2001]

Please provide a reproduction project. The exact setup you're using is necessary to reproduce the issue. It sounds like you're using GDExtension which isn't mentioned at all in the steps to reproduce, and is far from trivial to setup.

I attached it now

[akien-mga]

Thanks, I can reproduce the issue that the Object count seems to keep increasing.

[kleonc]

This is an extended issue from #83339 opened by @BeayemX.

@Lazy-Rabbit-2001 Wrong issue linked?

---

I opened a new project with only a CharacterBody2D using template moving code, and a TileMaps with tiles. Every time the move_and_slide() is called and the collision between the body calling the method and another one that the body haven't collided with (if it's a TileMap, each piece of tile is considered as an individual collision body), the amount of collision increased.

The increasing Object count seems to be GodotPhysicsDirectBodyState2Ds being created whenever GodotBody2D::get_direct_state() is called for the first time for the given body. So it's one time thing for each body, it's not like 2 Objects will be created when different CharacterBody2Ds will collide with the same tile etc.

godot/scene/2d/physics_body_2d.cpp

Line 1120 in ba79e53

PhysicsDirectBodyState2D *bs = PhysicsServer2D::get_singleton()->body_get_direct_state(platform_rid);

godot/servers/physics_2d/godot_physics_server_2d.cpp

Line 1021 in ba79e53

return body->get_direct_state();

godot/servers/physics_2d/godot_body_2d.cpp

Lines 734 to 740 in ba79e53

	GodotPhysicsDirectBodyState2D *GodotBody2D::get_direct_state() {
	if (!direct_state) {
	direct_state = memnew(GodotPhysicsDirectBodyState2D);
	direct_state->body = this;
	}
	return direct_state;
	}

Not sure if that's a bug, such created GodotPhysicsDirectBodyState2D is cleared in the GodotBody2D's destructor:

godot/servers/physics_2d/godot_body_2d.cpp

Lines 750 to 757 in ba79e53

	GodotBody2D::~GodotBody2D() {
	if (fi_callback_data) {
	memdelete(fi_callback_data);
	}
	if (direct_state) {
	memdelete(direct_state);
	}
	}

[Lazy-Rabbit-2001]

This is an extended issue from #83339 opened by @BeayemX.

@Lazy-Rabbit-2001 Wrong issue linked?

---

I opened a new project with only a CharacterBody2D using template moving code, and a TileMaps with tiles. Every time the move_and_slide() is called and the collision between the body calling the method and another one that the body haven't collided with (if it's a TileMap, each piece of tile is considered as an individual collision body), the amount of collision increased.

The increasing Object count seems to be GodotPhysicsDirectBodyState2Ds being created whenever GodotBody2D::get_direct_state() is called for the first time for the given body. So it's one time thing for each body, it's not like 2 Objects will be created when different CharacterBody2Ds will collide with the same tile etc.

godot/scene/2d/physics_body_2d.cpp

Line 1120 in ba79e53

PhysicsDirectBodyState2D *bs = PhysicsServer2D::get_singleton()->body_get_direct_state(platform_rid);

godot/servers/physics_2d/godot_physics_server_2d.cpp

Line 1021 in ba79e53

return body->get_direct_state();

godot/servers/physics_2d/godot_body_2d.cpp

Lines 734 to 740 in ba79e53

	GodotPhysicsDirectBodyState2D *GodotBody2D::get_direct_state() {
	if (!direct_state) {
	direct_state = memnew(GodotPhysicsDirectBodyState2D);
	direct_state->body = this;
	}
	return direct_state;
	}

Not sure if that's a bug, such created GodotPhysicsDirectBodyState2D is cleared in the GodotBody2D's destructor:

godot/servers/physics_2d/godot_body_2d.cpp

Lines 750 to 757 in ba79e53

	GodotBody2D::~GodotBody2D() {
	if (fi_callback_data) {
	memdelete(fi_callback_data);
	}
	if (direct_state) {
	memdelete(direct_state);
	}
	}

sorry, the link does be wrong and I fixed it just now
as for the clearness, there seems to be something that prevents the state from being destructed...
(sorry, I haven't learnt well how these codes work and this is just a simple track on my point of view

[Lazy-Rabbit-2001]

Not sure if that's a bug, such created GodotPhysicsDirectBodyState2D is cleared in the GodotBody2D's destructor:

godot/servers/physics_2d/godot_body_2d.cpp

Lines 750 to 757 in ba79e53

	GodotBody2D::~GodotBody2D() {
	if (fi_callback_data) {
	memdelete(fi_callback_data);
	}
	if (direct_state) {
	memdelete(direct_state);
	}
	}

I mentioned an issue that the direct state can get deleted only when the body is removed from the memory, but what if the body is running?
I think there seems to lack a way to remove previous direct state to ensure the RAM is guarded from leaking.
(Due to my busy life I haven't enough time to look through the relative cpp files to certify if the bug does happen because of the lack of removing stored direct state in the runtime of the body, so this is just my own PoV on this problem)

[Lazy-Rabbit-2001]

Seems that the pr #90668 may fix this issue, as I saw that the pr switched pointer to ObjectID that may seem to solve the problem of making RefCounted being undeletable.
I know almost nothing about how the low-level codes work, but this pr gave me that image.

Once the pr gets merged and another test for the bug, which will be conducted in the version where the feature of that pr gets implemented, does not fail freeing the KinematicCollisions, this topic will get closed