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

CharacterBody3D / Garbage Collector Memory Leak #53172

Closed
Tracked by #45333
kalysti opened this issue Sep 28, 2021 · 14 comments
Closed
Tracked by #45333

CharacterBody3D / Garbage Collector Memory Leak #53172

kalysti opened this issue Sep 28, 2021 · 14 comments

Comments

@kalysti
Copy link
Contributor

kalysti commented Sep 28, 2021

Godot version

4.0.dev (mono)

System information

Windows10

Issue description

Memory Leak for CharacterBody3D. Colliding not releasing objects.

image

Steps to reproduce

  • Use mono
  • Create a CharacterBody3D
  • Set GlobalTransform to any Position
  • Wait for colliding -> Let him fall down, watch the holded objects on the engine. -> Tada Memory leak

Minimal reproduction project

No response

@kalysti
Copy link
Contributor Author

kalysti commented Sep 28, 2021

Related mby with #50300

@akien-mga akien-mga added this to the 4.0 milestone Sep 28, 2021
@kalysti
Copy link
Contributor Author

kalysti commented Sep 28, 2021

Important notice: Just happend on mono. GDScript works well. Looks like a Garbage Collector Issue. Related mby to my previous issue. #53156

@kalysti
Copy link
Contributor Author

kalysti commented Sep 28, 2021

Okai interestening in mono i have to Dispose the MoveAndCollide Object to free the memory (thats new).. So its a bug,but can be fixed like this

public override void _PhysicsProcess(float delta) { var test = MoveAndCollide(Vector3.Down); if(test != null) test.Dispose(); }

@kalysti
Copy link
Contributor Author

kalysti commented Sep 28, 2021

So im pretty sure the garbage collector is not working in a right way.

@kalysti
Copy link
Contributor Author

kalysti commented Sep 28, 2021

public override void _PhysicsProcess(float delta){ MoveAndSlide(); }
produce a memory leak. Because cant be dispose

@kalysti kalysti changed the title CharacterBody3D Memory Leak CharacterBody3D / Garbage Collector Memory Leak Sep 28, 2021
@kalysti
Copy link
Contributor Author

kalysti commented Sep 28, 2021

Simple fix
MoveAndSlide(); if (GetSlideCollisionCount() > 0) { var obj = GetSlideCollision(0); if (obj != null) obj.Dispose(); }

Needs to be describe in the documentation.

@akien-mga
Copy link
Member

akien-mga commented Sep 28, 2021

You shouldn't have to dispose this manually. Mono is garbage collected, and this should be handled by the GC. If it's not working, that's a bug, not something that should be worked around manually by all users. There's no point using a GC-ed language if the GC doesn't work :)

@neikeq
Copy link
Contributor

neikeq commented Sep 28, 2021

public override void _PhysicsProcess(float delta){ MoveAndSlide(); } produce a memory leak. Because cant be dispose

Is this all that's needed to reproduce the issue?

@neikeq
Copy link
Contributor

neikeq commented Sep 28, 2021

I have not tested yet, so I can't tell if there's a leak or not. However, the garbage collector doesn't necessarily collect resources right away. There were similar issues to this in the past, and the conclusion was that the GC simply didn't collect them yet.

One way to check if this is the case is to try to force collection this way somewhere like in _Process:

GC.Collect(GC.MaxGeneration);
GC.WaitForPendingFinalizers();

If doing that doesn't fix the issue, then there may be a leak.

Keep in mind, I only recommend that doing that for debugging purposes. In practice, if you want an object to be disposed right away instead of leaving it to the GC, then you have to call Dispose() as there's no RAII in C#.

@pouleyKetchoupp
Copy link
Contributor

I wonder if it's a side effect of this change: #52954

I'm not re-using the cache if there's still a reference, but maybe there's an extra reference in mono that causes this logic to create a new object every time?

@kalysti
Copy link
Contributor Author

kalysti commented Sep 29, 2021

Today i will dive a bit deeper. But the issue is not a physics issue. Its 100% a mono issue. The problem occurs on many different points. See also my previous issue ticket one day ago (same problem, different context). For some reasons the garbage collector does not wach over each holded object. So seems like to be a bigger issue. i blv the easiest way to detect the issue is to build a previous commit (mby 14days ago, i see there was some several mono changes), and resimulate the problem. But im defently sure before my big baby timeout (yeah im father), 3 months ago it was working well. Also when i was writing the terrain tool the garbage collector was working. When i dispose the objects manualy i can prevent the memory leak. But for sure its a dirty solution.

@kalysti
Copy link
Contributor Author

kalysti commented Sep 29, 2021

I have not tested yet, so I can't tell if there's a leak or not. However, the garbage collector doesn't necessarily collect resources right away. There were similar issues to this in the past, and the conclusion was that the GC simply didn't collect them yet.

One way to check if this is the case is to try to force collection this way somewhere like in _Process:

GC.Collect(GC.MaxGeneration);
GC.WaitForPendingFinalizers();

If doing that doesn't fix the issue, then there may be a leak.

Keep in mind, I only recommend that doing that for debugging purposes. In practice, if you want an object to be disposed right away instead of leaving it to the GC, then you have to call Dispose() as there's no RAII in C#.

Thanks for the tip. I will reproduce it later and try to manualy trigger the GC. I will inform you about the result.

@kalysti
Copy link
Contributor Author

kalysti commented Sep 29, 2021

I have not tested yet, so I can't tell if there's a leak or not. However, the garbage collector doesn't necessarily collect resources right away. There were similar issues to this in the past, and the conclusion was that the GC simply didn't collect them yet.

One way to check if this is the case is to try to force collection this way somewhere like in _Process:

GC.Collect(GC.MaxGeneration);
GC.WaitForPendingFinalizers();

If doing that doesn't fix the issue, then there may be a leak.

Keep in mind, I only recommend that doing that for debugging purposes. In practice, if you want an object to be disposed right away instead of leaving it to the GC, then you have to call Dispose() as there's no RAII in C#.

To trigger the gc manualy like u described above, solves the leak.

So yeah we have an memory leak inside the garbage collector.

@kalysti
Copy link
Contributor Author

kalysti commented Jun 27, 2022

Godot version

4.0.dev (mono)

System information

Windows10

Issue description

Memory Leak for CharacterBody3D. Colliding not releasing objects.

image

Steps to reproduce

  • Use mono
  • Create a CharacterBody3D
  • Set GlobalTransform to any Position
  • Wait for colliding -> Let him fall down, watch the holded objects on the engine. -> Tada Memory leak

Minimal reproduction project

No response

I think this issue is fixed. You can close it

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

4 participants