godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.21k stars 21.21k forks source link

Memory leak when using `get_last_slide_collision()` #57073

Open BeayemX opened 2 years ago

BeayemX commented 2 years ago

Godot version

v4.0.dev.custom_build [b5f524d4c]

System information

Linux Mint - Vulkan API 1.2.131 - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce GTX 1080

Issue description

Using get_last_slide_collision() causes object to be created but never deleted.

This can be seen in the object count in the profiler.

Steps to reproduce

Use a CharacterBody2D and use get_last_slide_collision() when a collision happenes.
Check with Performance.get_monitor(Performance.OBJECT_COUNT) or the profiler in the editor that the object count is only increasing.

OR

Run the attached project to see the issue with everything prepared.

Minimal reproduction project

  1. The player (white square) will fall and collide with the static collider (white rectangle).
  2. In the UI you can see the current number of existing objects
  3. Press space to jump
  4. Notice that the number of objects will not increase as long as the player is NOT colliding with the ground

MemoryLeak_get_last_slide_collision.zip

markdibarry commented 2 years ago

I'm guessing the root of the problem is right here where we create a new copy of the MotionResult and never free old ones. image

LeaoLuciano commented 2 years ago

https://github.com/godotengine/godot/blob/050908626f64c0c984e078055215c8b5f6231ce3/modules/gdscript/gdscript_vm.cpp#L1819-L1823 Looking to backtraces on gdb, ret_opaque is a pointer to a Ref\<KinematicCollision2D> 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: https://github.com/godotengine/godot/blob/050908626f64c0c984e078055215c8b5f6231ce3/modules/gdscript/gdscript_vm.cpp#L3353-L3355.

Lazy-Rabbit-2001 commented 1 year ago

Sorry, but I don't know if it's fittable to talk it here: I opened a test project with a CharacterBody2D (with a template script for CharacterBody2D) and a TileMap, and no get_last_slide_collision() used (even no get_slide_collision()), but the amount of objects got increased every time i made the body collide with a piece of tile(Caution: it's A PIECE of tile, not the whole tile)

I think a probable fix is to clear all old elements in Vector<Ref<KinematicCollision2D>> slide_colliders in the beginning part of code of move_and_slide()