2

I have a .NET Core C# class that wraps an unmanaged pointer and it should be freed on program exit along with other resource cleanup. However, the destructor is not being called. I have tried in both Debug and Release mode. I see that .NET Core apparently doesn't guarantee that destructors will be run, so what is a recommended workaround? IMO the main point of garbage collection is to avoid having the developer track references, so I find this behavior surprising, to say the least.

From MSDN: In .NET Framework applications (but not in .NET Core applications), finalizers are also called when the program exits.

public Demo { 
  IntPtr _ptr;

  public Demo() 
  { 
    Console.WriteLine("Constructor");
    _ptr = /* P-invoke external function */ 

  ~Demo 
  {
    Console.WriteLine("Destructor");
    /*P-invoke ptr deletion */
  }
}

public static void Main() 
{ 
  Demo demo = new Demo();
  demo = null;
  GC.Collect();
}

Program output:

Constructor
<...>\Test.exe (process 7968) exited with code 0.
Ry-
  • 218,210
  • 55
  • 464
  • 476
evo8198
  • 129
  • 10
  • 2
    Here are some explanations https://stackoverflow.com/questions/44732234/why-does-the-finalize-destructor-example-not-work-in-net-core – CharithJ Mar 30 '21 at 00:37
  • Have you tried to declare `Demo` as a static Field: `static Demo demo = new Demo();`, then setting it to `null` and calling `GC.Collect()`? Do you mean to implement `IDisposable` here? (what version of .Net Core?) – Jimi Mar 30 '21 at 00:46
  • IMO a finalizer being run is a bug. Use a `using` block with `IDisposable` to guarantee disposal as far as possible. Remember that catastrophic failure is always a problem eg `Environment.FailFast`, `taskkill` or a power failure – Charlieface Mar 30 '21 at 00:47
  • @Jimi, using a static variable is a complete hack. We have local variables because having everything global is poor coding. Yes, IDisposable does fix the issue. – evo8198 Mar 30 '21 at 00:52
  • @Charlieface, I don't understand your statement that "a finalizer being run is a bug". Automatic cleanup is deemed useful by many - see smart pointers and .Net Framework behavior, for example. – evo8198 Mar 30 '21 at 00:56
  • I didn't say or suggested to *use it*, I suggested to *test it*. What you do, in practice, is your decision only. Do you need to dispose of something in that class? Why are you not talking about `IDisposable`? -- Not clear what this means: *We have local variables because putting all instances in local variables is bad coding style*. Did you choose *bad coding style*, whatever that means here? -- All right, the edit makes that statement more understandable. – Jimi Mar 30 '21 at 00:56
  • _having everything global is poor coding_ - is true for **everything**, but having global static values is totally normal - _right tool for the job_ :) – Fabio Mar 30 '21 at 00:57
  • 1
    Automatic cleanup of unmanaged resources is a laudable goal, but notoriously difficult to achieve in C#, and finalizers and not the answer to this. They are absolutely not guaranteed to run, it's best efforts only. Further reading http://joeduffyblog.com/2005/04/08/dg-update-dispose-finalization-and-resource-management/ and https://www.viva64.com/en/b/0437/ and https://scale-tone.github.io/2018/04/13/my-favourite-catastrophic-failures-finalizers and https://www.codeproject.com/Articles/15360/Implementing-IDisposable-and-the-Dispose-Pattern-P – Charlieface Mar 30 '21 at 01:06
  • 3
    Other than demo code, I've never written a Finalizer in my 20 years of .NET programming (I started with the _alpha_ version of C#). Finalizers are hard to write properly, extremely hard to test, and even when you write them, really should never run (you should do your cleanup using `IDisposable` whenever possible). If you have a absolute requirement, look at `SafeHandle`s – Flydog57 Mar 30 '21 at 01:22

2 Answers2

3

More changes are needed to improve the likelihood that the finalizer will be called.

Btw, Finalizer is never guaranteed to be called. If you want to gurantee the resources release, implement IDisposable and call Dispose() before the app/method/code block exit. Additionally to make Dispose() guaranteed to call (even if app crashes, except FailFast and StackOverflow) before exiting the code block, use try-finally or using statements.

Here's an example to play with.

public class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine("[main] Constructing");
        MyDisposable m = new MyDisposable(0);
        MyMethod(1);
        Console.WriteLine("[main] Disposing [object 0]");
        m.Dispose();
        Console.WriteLine("[main] GC Collecting");
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Console.WriteLine("[main] Done");
        Console.ReadKey();
    }

    private static void MyMethod(int i)
    {
        new MyDisposable(i);
    }
}

public class MyDisposable : IDisposable
{
    private int _id;

    public MyDisposable(int id)
    {
        _id = id;
        Console.WriteLine($"[object {_id}] Constructed");
    }

    private bool disposed = false;

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            {
                Console.WriteLine($"[object {_id}] Disposing by Dispose()");
            }
            else
            {
                Console.WriteLine($"[object {_id}] Disposing by ~Finalizer");
            }
            Console.WriteLine($"[object {_id}] Disposed");
            disposed = true;
        }
        else
            Console.WriteLine($"[object {_id}] Already disposed!");
    }

    ~MyDisposable()
    {
        Dispose(false);
    }
}

Output

[main] Constructing
[object 0] Constructed
[object 1] Constructed
[main] Disposing [object 0]
[object 0] Disposing by Dispose()
[object 0] Disposed
[main] GC Collecting
[object 1] Disposing by ~Finalizer
[object 1] Disposed
[main] Done

Some read: Using objects that implement IDisposable.

aepot
  • 4,558
  • 2
  • 12
  • 24
  • `using` with `IDisposable` calls `Dispose()`, with or without `GC.WaitForPendingFinalizers()`. Also, `GC.WaitForPendingFinalizers()` without `IDisposable` does not force the finalizer to be run - I'm not clear on what exactly you thought it would do. – evo8198 Mar 30 '21 at 01:08
  • 1
    **Finalizer is never guaranteed to be called.** should be double bold. @evo8198 The point here is that if you want to dispose, use `IDisposable`. Your second point is simply not true. Finalizers have nothing intrinsically to do with `IDisposable`, other than their code usually just calls through to `IDisposable.Dispose`, which is why `Dispose` should contain `GC.SuppressFinalize` – Charlieface Mar 30 '21 at 01:12
  • @evo8198 i just showed how to make GC to call `~Demo`. Just for example. But I didn't tell anything about connection between GC methods explicit calls and `IDisposable`. – aepot Mar 30 '21 at 01:15
  • @aepot, in my comment I was stating that `GC.WaitForPendingFinalizers()` does not actually force `~Demo()` be called. I just tested it. – evo8198 Mar 30 '21 at 01:20
  • `Additionally to make Dispose() guaranteed to call` It isn't _guaranteed_ to call. e.g. an exception on .NET Core on Linux may mean the `Dispose` isn't called. https://github.com/dotnet/runtime/issues/8145#issuecomment-302772885 – mjwills Mar 30 '21 at 01:26
  • `Finalizers have nothing intrinsically to do with IDisposable, other than their code usually just calls through to IDisposable.Dispose` @Charlieface The docs / MS pattern certainly do not suggest that finalizers call `IDisposable.Dispose`. This, generally speaking, is a terrible idea. – mjwills Mar 30 '21 at 01:29
  • @evo8198 added more complex example – aepot Mar 30 '21 at 01:33
  • @mjwills, Release mode. I saw some other posts indicating that Debug can hold onto object references. – evo8198 Mar 30 '21 at 01:33
  • @mjwills Apologies `Dispose(false)` is what I meant – Charlieface Mar 30 '21 at 01:34
  • @aepot, are you using .NET Core or .NET Framework? – evo8198 Mar 30 '21 at 01:49
  • @evo8198 sorry, missed that was a Framework. Fixed the example for .NET Core 3.1/.NET 5 to align with output. Tested both, Debug and Release, output now matched shown in the answer. Collectable object must be out of visible scope (just to reproduce ~Finalizer call). – aepot Mar 30 '21 at 02:00
  • 2
    @aepot, thanks, that was the issue - finalizer was not being called for me because I was calling the GC methods in the same scope as the object. I expected setting the object to null would be sufficient, but it is not. – evo8198 Mar 30 '21 at 02:13
1

The official Cleaning up unmanaged resources states:

If your types use unmanaged resources, you should do the following:

Implement the dispose pattern. (...)

In the event that a consumer of your type forgets to call Dispose, provide a way for your unmanaged resources to be released. There are two ways to do this:

  • Use a safe handle to wrap your unmanaged resource. This is the recommended technique. Safe handles are derived from the System.Runtime.InteropServices.SafeHandle abstract class and include a robust Finalize method. When you use a safe handle, you simply implement the IDisposable interface and call your safe handle's Dispose method in your IDisposable.Dispose implementation. The safe handle's finalizer is called automatically by the garbage collector if its Dispose method is not called.

(...)

Implement a Dispose method contains:

  • 'the general pattern for implementing the dispose pattern for a base class that uses a safe handle'; and
  • 'the general pattern for implementing the dispose pattern for a base class that overrides Object.Finalize'.
tymtam
  • 31,798
  • 8
  • 86
  • 126