Nate Diamond
Nate Diamond

Reputation: 5575

private void Dispose(bool)?

In a couple of places, people have suggested to use private void Dispose(bool) for the IDisposable pattern. This seems outdated though (at least for unsealed classes), as the new suggested pattern (according to Microsoft) is protected virtual void Dispose(bool).

The thing is, Code Analysis does not report private void Dispose(bool) for violating CA1063, even though it seems to violate the pattern directly.

What's up with this? Is private void Dispose(bool) somehow getting called (or compiled to something that looks like protected virtual Dispose(bool)?

If this is some issue with Code Analysis and is the incorrect pattern, are there ways to detect this? Possibly with StyleCop?

Edit: After consideration, is it that a base class can call base.Dispose() which will hit private void Dispose(bool)? Even if it isn't able to pass in an argument?

Edit: Sample

public class A : IDisposable
{
    ~A()
    {
        this.Dispose(false);
    }

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

    private void Dispose(bool disposing) // Should be protected virtual void Dispose(bool)
    {
        Console.WriteLine("A");
    }
}

public class B : A
{
    protected virtual void Dispose(bool disposing) // Proper pattern.
    {
        Console.WriteLine("B");
    }
}

public static class Program
{
    static void Main(string[] args)
    {
        A a = new A();
        a.Dispose(); // Prints "A"

        B b = new B();
        b.Dispose(); // Prints "A"!
    }
}

As you can see from this, it makes using the dispose pattern totally unwieldy.

You can get around this a bit by hiding the public void Dispose(void) and then calling base.Dispose() somewhere. This then works 'similar' to the proper dispose pattern when calling B b = new B(); b.dispose(); except when calling A b = new B(); b.Dispose();, which only calls A's Dispose method.

public class B : A
{
    public void Dispose() // Causes CA error with or without "new".
    {
        this.Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing) // Proper pattern.
    {
        base.Dispose(); // Writes "A" (without quotes).
        Console.WriteLine("B");
    }
}

Basically, this whole thing seems terrible. Do we know if it is a bug that CA accepts private void Dispose(bool) and is there a way to at least throw a warning with StyleCop?

Edit: I don't think I should accept Alexandre's answer, as relative to the question I have it basically boils down to "Could be a bug", along with something that should be a comment. If anyone else has something more conclusive, I think that would be a more appropriate answer.

Upvotes: 5

Views: 1307

Answers (1)

Alexandre Borela
Alexandre Borela

Reputation: 1616

Implementing a Dispose Method

The IDisposable interface requires the implementation of a single parameterless method, Dispose. However, the dispose pattern requires two Dispose methods to be implemented:

  • A public non-virtual (NonInheritable in Visual Basic) IDisposable.Dispose implementation that has no parameters.
  • A protected virtual (Overridable in Visual Basic) Dispose method.

Because the public, non-virtual (NonInheritable in Visual Basic), parameterless Dispose method is called by a consumer of the type, its purpose is to free unmanaged resources and to indicate that the finalizer, if one is present, doesn't have to run. Because of this, it has a standard implementation:

public void Dispose()
{
   // Dispose of unmanaged resources.
   Dispose (true);
   // Suppress finalization.
   GC.SuppressFinalize (this);
}

In the second overload, the disposing parameter is a Boolean that indicates whether the method call comes from a Dispose method (its value is true) or from a finalizer (its value is false).

When the garbage collector decides that your object is no longer needed, it'll try to finalize it in case you forgot to call the parameterless dispose method, because if you did and you are following the pattern, the call would be suppressed.

See: How Finalization Works

Private vs Protected Virtual:

You should always use protected virtual as the documentation says if you ever want to support subclasses that follows the pattern correctly.

Why some people use the private version? Maybe because inheritance was never their intention, specially if you are just generating methods on the fly using tools like Resharper, most of the time these methods are going to be private.

Why Code Analysis doesn't report the problem?

Could be a bug. Provide a small sample that gives the problem so that other people could test on their machines.

Upvotes: 6

Related Questions