search

dotnet / docs

This repository contains .NET Documentation.
https://learn.microsoft.com/dotnet
Creative Commons Attribution 4.0 International
4.17k stars 5.83k forks source link

Conflicting advice about using finalizers to clear up unmanaged resources #28946

Closed wgibbons1 closed 1 year ago

wgibbons1 commented 2 years ago

My understanding is that if we use unmanaged resources, we must manually free them up or they will persist after application termination. If this is correct, then the advice to use finalizers to do this (first two quotes below) is wrong as we can't be sure they will run in Core (third quote below).

"Even with this explicit control over resources, the finalizer becomes a safeguard to clean up resources if the call to the Dispose method fails." and "However, when your application encapsulates unmanaged resources, such as windows, files, and network connections, you should use finalizers to free those resources. " conflict with "Because you can't call Finalize directly, and you can't guarantee the garbage collector calls all finalizers before exit, you must use Dispose or DisposeAsync to ensure resources are freed."

The document feels slightly ambiguous regarding exactly what we should do to ensure unmanaged resources are freed up correctly. Seems as if there should be a standard pattern for Framework and another for Core.

The first quote doesn't seem right to me as I wouldn't call it a safeguard if it is not certain it will happen (in Core).

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

BillWagner commented 2 years ago

Hi :wave: @wgibbons1

Thanks for reaching out. I'd like to clarify some points before we make any changes in this article.

My understanding is that if we use unmanaged resources, we must manually free them up or they will persist after application termination. If this is correct, then the advice to use finalizers to do this (first two quotes below) is wrong as we can't be sure they will run in Core (third quote below).

That isn't correct. On the operating systems supported by .NET Core (and .NET 5 and newer), the operating system will free any unmanaged resources when the application exits. The purpose of a finalizer is to free any unmanaged resources before application exit, and only if those resources weren't freed by a Dispose method.

The document feels slightly ambiguous regarding exactly what we should do to ensure unmanaged resources are freed up correctly. Seems as if there should be a standard pattern for Framework and another for Core.

The standard pattern should be the same. In any application, you should free any unmanaged resources by using a finalizer (or using SafeHandle<T> which creates the finalizer for you). That ensures your application is well behaved. The runtime can't guarantee finalizers are called when the application exits. (The OS may force quit any application, without giving it a chance to clean up.) The OS then takes over all cleanup.

In light of that, what changes would you recommend to improve the clarity here?

wgibbons1 commented 2 years ago

Hi Bill,

Thanks for the reply.

In that case my suggestions would change slightly.

Because you can't call Finalize directly, and you can't guarantee the garbage collector calls all finalizers before exit, you must use Dispose or DisposeAsync to ensure resources are freed.

This sentence says to me that if you don't call Dispose() then it will not be certain that resources will be freed. As an extension, readers who are not aware that the OS clears all resources on app termination will then be guessing that if a call to Dispose() is forgotten then the resources will remain allocated even after app termination.

To tackle both these issues I think it could be explained that no matter what happens in the app, the resources will be cleared on app termination by the OS, and that Dispose() only needs to be used to ensure resources are freed immediately.

I also have a question about the preceding text in the paragraph:

If you need to perform cleanup reliably when an application exits, register a handler for the [System.AppDomain.ProcessExit] event. That handler would ensure IDisposable.Dispose() or IAsyncDisposable.DisposeAsync() has been called for all objects that require cleanup before application exit.

If the OS cleans up automatically then why would we need to do this on app exit? Presumably you mean stuff the OS doesn't do, which for example could relate to application logic, like resetting a user config file (can't think of a better example)? If so then I think this distinction needs to be mentioned too (for mere mortals like myself).

William

BillWagner commented 2 years ago

Thanks @wgibbons1

I think most of the answers are in this section of the docs. Let me know if those do answer these questions. If so, we'll make that link more prominent in this article.

Thanks.

github-actions[bot] commented 1 year ago

This issue has been automatically closed due to no response from the original author. Please feel free to reopen it if you have more information that can help us investigate the issue further.