Implementing IDisposable correctly on parent classes in C#

I have a class which implements the C# SerialPort which used to look like this:

public class AsyncSerial : IDisposable
{
    SerialPort newPort; //Parameters declared in my constructor
    //Constructor and other methods

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

    protected virtual void Dispose(bool disposing)
    {
        if(disposing)
        {
            this.Close();
            this.Dispose();
        }
    }
}

This throws up no warnings in code analysis (I got the code from MSDN as an example of how to do it properly).

Since I was only ever going to declare one SerialPort I figured I would make my class a child of SerialPort, but now I get warnings that I can't seem to fix.

public class AsyncSerial : SerialPort
{
    //Constructor and other methods

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

    protected new virtual void Dispose(bool disposing)
    {
        if(disposing)
        {
            this.Close();
            this.Dispose();
        }
    }
}

Code warnings said the dispose methods should be new as they hide members, which I did, but I also get:

"Warning CA1063 Ensure that 'AsyncSerial.Dispose()' is declared as public and sealed"

Making it sealed means it has to be marked override (or I get compiler errors), making it override means it can be new, so I end up with:

Error CS0506 'AsyncSerial.Dispose()': cannot override inherited member 'Component.Dispose()' because it is not marked virtual, abstract, or override

I don't know the 'correct' way to implement disposing on a class with IDisposable in the parent class. Every example I find only fits having IDisposable as a base, but making my class

public class AsyncSerial : SerialPort, IDisposable
{
    //code
}

gives me a code analysis warning because SerialPort already implements IDisposable.

Should I just suppress the warning about Ensuring that 'AsyncSerial.Dispose()' is declared as public and sealed, or is there a correct way to do this which doesn't give code analysis warning.

Jon Skeet
people
quotationmark

Your subclass should override Dispose(bool disposing) if anything - that's the whole point of having that method at all, really.

However, I suspect that the base class will make the right calls anyway, so you shouldn't need to do anything, unless you have extra resources to release which aren't released in Close(). If that's the case, do that in Dispose(bool disposing):

protected override void Dispose(bool disposing)
{
    // Allow the base class to release resources
    base.Dispose(disposing);
    // Release any extra resources here 
}

Note that your current implementation will lead to a StackOverflowException as your two Dispose overloads call each other.

people

See more on this question at Stackoverflow