FosterFramework / Foster

A small C# game framework
MIT License
435 stars 38 forks source link

Finalize/Dispose patterns #32

Closed MrBrixican closed 10 months ago

MrBrixican commented 10 months ago

We probably want to standardize/adjust the way we handle unmanaged resource lifecycles.

.NET makes no guarantee that finalizers will actually run on application exit.

Currently, the following classes implement the dispose/finalize pattern: Mesh, Shader, Target, Texture, Batcher, Font, Image

Mesh, Shader, Target, Texture are specifically graphics resources, which we ideally want to ensure get disposed before the application exits. Monogame/FNA track them with weak references and dispose them on application exit. Additionally, FNA will keep track when a finalizer is hit during run and delay release calls until it can guarantee it will be called on the main thread during present.

Batcher, Font, and Image aren't as significant since they just point to blocks of memory, which should be freed automatically on application termination anyways.

We should also look into using GC.SuppressFinalize where possible to further reduce true object lifetime.

NoelFB commented 10 months ago

Interesting! Yeah I didn't consider finalizers not being called on application exit. I do currently delay dispose for graphical resources until it's on the main thread since the finalizers can run off-thread, but we should fix the other issues and maybe streamline that at the same time. I can look into this.

NoelFB commented 10 months ago

Alright, I've pushed an implementation but leaving this open until it's been reviewed a little more. Basically it should ensure that all resources are 1) properly deleted and 2) their managed objects are also marked as Disposed, so even if you run the Application multiple times and reference managed objects from before, they will no longer think that they're still valid.

MrBrixican commented 10 months ago

Awesome! Looks great.

NoelFB commented 10 months ago

This seems like it's working well in all my stuff, so closing!