Dispose Method Implements IDisposable

ID

csharp.dispose_method_implements_idisposable

Severity

critical

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

api-design, idisposable, reliability, resource-leak

Description

Reports a method named Dispose on a class, struct or record that does not implement IDisposable. The name promises the framework disposal contract, but nothing in the runtime or the language will honour it.

Rationale

In .NET, Dispose is not an ordinary method name. It is the single method of IDisposable, and the language and the framework are both built around that fact: using blocks and using declarations call it at the end of scope, foreach calls it on enumerators, dependency-injection containers call it when a scope ends, and collection types call it on the items they own. All of that dispatches through the interface. A Dispose method on a type that does not implement IDisposable receives none of those calls.

The consequences are asymmetric and easy to miss. The type cannot be used in a using block at all — that is a compile error, which at least surfaces immediately. Worse is the silent case: a caller sees Dispose and reasonably assumes the framework is calling it, so nobody calls it explicitly, and the file handle, socket or unmanaged buffer it was meant to release is never released. The leak shows up much later, far from this type.

The reverse reading is just as bad: if the method is not about releasing resources, its name misleads every reader into thinking the type owns something that must be cleaned up.

The standard Dispose(bool disposing) pattern is not affected. In that pattern the type declares IDisposable, so neither Dispose() nor the protected Dispose(bool) overload is reported.

Only a type that declares no base list at all is reported, because that is the case where the absence of IDisposable is provable from the file being analysed. A base class or a base interface may itself derive from IDisposable — IEnumerator does — without that being visible here, so a type naming any base type is left alone. partial types are skipped for the same reason: another part may carry the base list.

An extension method — its first parameter carries the this modifier — is never reported, regardless of the declaring type’s base list. Its semantic target is the extended parameter’s type, not the declaring (necessarily static) type, and nobody expects a static extension method to be invoked implicitly: callers write collection.Dispose() explicitly and get exactly what the extension method documents.

using System;

public class Buffer
{
    private byte[] data = new byte[1024];

    public void Dispose()                       // FLAW — no IDisposable, so nothing calls this
    {
        data = null;
    }
}

public class Connection : IDisposable
{
    public void Dispose()                       // OK — the interface implementation
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)   // OK — standard dispose pattern
    {
    }
}

public class Session
{
    public void Close()                         // OK — a name that promises nothing
    {
    }
}

public static class IEnumerableExtensions
{
    public static void Dispose<T>(this IEnumerable<T> items)  // OK — extension method,
        where T : IDisposable                                //     targets T, not this class
    {
        foreach (var item in items) item?.Dispose();
    }
}

Limitations

The check does not resolve the base list at all, in this file or any other: a type naming any base class or interface is left alone unconditionally, even when that supertype is declared right here and provably does not implement IDisposable either. In practice the types most likely to carry this bug are the ones that already implement some other interface or extend some other base — a class that was sealed and stand-alone rarely needs a Dispose method added to it by mistake — so the rule’s blind spot lands on the more common real-world shape rather than an edge case.

Remediation

Decide which of the two contracts the type actually has.

  • The type owns resources that must be released: declare IDisposable (and IAsyncDisposable when the release is asynchronous), keep Dispose() public and parameterless, and adopt the Dispose(bool disposing) pattern if the type is unsealed. Callers then get using support and automatic disposal from every framework component that owns the object.

  • The method does something else: rename it to say what it does — Close, Reset, Clear, Release — so no reader infers a disposal contract that is not there.

References