Why disposing NativeArray in a finalizer is unacceptable?

I have tried using NativeArray and Unity error is great at catching memory leaks. However it is getting out of hand when the class that is housing NativeArray is being in the other class in a big chain and turns out everything must implement `IDisposable` and have to remember to `.Dispose`. Things would be better if I had done ECS but a transition is needed.

Having doing things in OOP all along there are so many places that I just let a newly created class instance goes out of scope for the GC to pick it up, thus left NativeArray dangling and Unity logs error.

I have an idea to put all the NativeArray.Dispose() in the class's finalizer (~YourClass()) this way I would not have to propagate `IDisposable` to everything. Let the class instance variable being GC collected like usual and from there dispose all the insides. I log the finalizer method and it did get called when GC collects the class instance, however Unity checks and throws memory leak error one step before the finalizer call.

So I want to ask what leads to this design decision? Could we check for memory leak after the finalizer?

Also as a suggestion, the "It was allocated at line xxx" message is useful but some possible improvements :

1. Can we show something or some number which relates to an instance that is getting release causing memory leak. In my game when it shows "It was allocated at" they are all the same since I have only one place that creates NativeArray, however I want to at least see some differences between those message and see if how many of them came from the same instance release. I might be able to figure out something more. (More than following all of the possible usage of that class)
2. NativeArray allocated in a class field initializer show up on memory leak as "was allocated at line 0:" without file name. Can we at least show a file name?

 

It's not entirely a unity-specific thing. Finalizers in C# are actually kinda bad, and ARE NOT DETERMINISTIC AT ALL (as in, unlike a C++ destructor, they don't get called immediately, they get called WHEVER THE .NET GC FEELS LIKE IT). This may or may not be after when unity runs it's leak checks..

The only way in .NET to get proper deterministic disposal is implementing the IDisposable interface in your container. Dispose can be called manually: here's the proper instructions from the MS guidelines.

It's probably better to just make the class holding the NativeArray have it's own Dispose method, and call it manually when you're done with the class, as that gets you determinism. If it's a MonoBehaviour or ScriptableObject, call dispose in OnDestroy(). Both options for normal C# classes and Unity script objects seems to work fine for me with Not-ECS code.

The irony here is that all the Dispose pattern does is basically do the same thing that C++ virtual destructors do, only with more boilerplate code and a kludge around the non-deterministic GC. It'd be hilarious if it wasn't sad, and it's one of the few issues I have with C# and .NET in general.

EDIT:
I forgot, they're also multithreaded, and ARE NOT GUARANTEED TO RUN ON THE MAIN THREAD.

Added bonus article:
https://www.viva64.com/en/b/0437/

 

Thank you!

Yesterday I watched my code turning into a bizarre version of C++ where I apply `using(){ }` and `.Dispose` band aid all over the place, now I use the class housing NativeArrays on a static space as much as possible so at least I am the one disposing them when changing scenes.

 

I feel this is more a misunderstanding of GC within C#. It's serving as a great tool for enterprise development that doesn't have to care too much about the same level of performance and allows for ease of development. `IDisposable` is meant for cases where the resources have to be managed. Unity being built with C# as the scripting language had meant that it created the expectation that GC would also work work with game development. There might be cases where it works, but the truth of the matter is it just doesn't always work as great.

ECS and especially the JobSystem is saying we're taking these training wheels off - the developer is becoming very much aware of the memory that they're allocating and it's becoming their responsibility to manage that as well. This is where the `IDisposable` interface does come in handy. That's why Unity included it in those containers. Feeling one has to "bubble" up the interface through all of your structures might be a misunderstanding of the intent in using the interface. It's more a question of understanding how long this object should live until it can be freed up again. Bubbling up only defers the responsibility of that instead of always fully understanding what is happening.

There are other problems with having this in the finalizer as well. As @recursive mentioned the Microsoft documentation has a suggested guideline regarding the pattern in conjunction with finalizers. Looking at it you'll note that the finalizer would be calling the `Dispose` method with false, which isn't taking care of disposing related `IDisposable` objects. That is because the GC could have already collected those objects. There's also a time limit to how long finalizer code can run if I remember correctly.

The best advice regarding the disposal of these container types is to keep their scope very small and limited. There are attributes you can use to mark a container for disposal once a job finishes, but I also think scheduling a cleanup job that can take care of it should suffice as well. At least then you won't be putting it in static variables where their scope could become global and you end up with a mess and you'll know that eventually everything should be taken care of.

 

Yeah, I had this great idea today where I thought "Hmmm ... I'll just dispose of these native collections in the destructor".

Well ... needless to say there's a reason why I found this thread. Thanks for the explanation. I'll just throw an exception in the destructor if Dispose() wasn't called by this point, just so users know exactly what they did wrong and where and how to fix it.