Dispose Members In Dispose

ID

csharp.dispose_members_in_dispose

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

api-design, idisposable, reliability, resource-leak

Description

Reports a type that calls Dispose() on one of its own fields from a method other than its Dispose method, when nothing reachable from Dispose releases that same field. Callers who honour the disposal contract call Dispose and nothing else, so a field released only elsewhere is leaked.

Rationale

The disposal contract in .NET has one entry point. using blocks, using declarations, foreach over an enumerator, dependency-injection scopes and every collection type that owns its items all converge on a single call to Dispose. Nothing calls Close, nothing calls Shutdown, and nothing guesses that a private Cleanup needs running. So when the code that releases a field lives outside Dispose and is not reachable from it, the field survives the one call the caller was ever going to make.

The failure is quiet and delayed. The object looks correctly disposed — the type implements the contract, the using block compiles, the review passes — while a file handle, a socket or a database connection stays open until the finaliser runs, if the field’s type even has one. Under load this is how a process runs out of handles or a connection pool starves, hours after the code that caused it ran.

The same reasoning applies to a disposable ref struct. A ref struct cannot implement IDisposable, and using binds to its Dispose method by shape rather than through an interface, so what matters is where the release actually happens, not what the base list says.

Reachability is judged per field. A Close, Reset or Abort that mirrors what Dispose already does — the usual .NET shape where both release the same handle — leaks nothing and is not reported; only a field that no path from Dispose or DisposeAsync releases is a finding. A private helper invoked from Dispose counts as part of the contract for the same reason. Finalisers are left alone as well, since releasing there is a distinct concern with its own trade-offs.

The subject is restricted to fields the type actually owns: non-static, private or protected, and of a type whose disposal is established (a well-known framework disposable, or a type in the same file whose base list names IDisposable / IAsyncDisposable). A public disposable field is assigned from outside, so its lifetime belongs to whoever assigned it and is explicitly compliant.

using System;
using System.IO;

public class Exporter : IDisposable
{
    private FileStream output = new FileStream("out.bin", FileMode.Create);
    private StreamWriter log = new StreamWriter("log.txt");

    public void Close()
    {
        output.Dispose();        // FLAW — a caller who only calls Dispose leaks output
    }

    public void Dispose()
    {
        log.Dispose();           // OK — released by the contract
    }
}

public class Session : IDisposable
{
    private FileStream fs = new FileStream("s.bin", FileMode.Create);

    public void Close()
    {
        fs.Dispose();            // OK — Dispose releases the same field, nothing leaks
    }

    public void Dispose()
    {
        fs.Dispose();
    }
}

public class Importer : IDisposable
{
    private FileStream input = new FileStream("in.bin", FileMode.Open);

    public void Dispose()
    {
        ReleaseAll();            // OK — the helper is reachable from Dispose
    }

    private void ReleaseAll()
    {
        input.Dispose();         // OK — Dispose calls this
    }
}

Remediation

Move the release into Dispose, or into a helper that Dispose calls. In the standard pattern that helper is Dispose(bool disposing), which both Dispose() and the finaliser delegate to, so every field is released exactly once regardless of which path ran.

If a method such as Close needs to stay for API reasons, make it delegate to Dispose rather than release fields itself. That keeps one release path and removes the possibility of a caller reaching only half of it.

References