No Return Using Disposable

ID

csharp.no_return_using_disposable

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

idisposable, object-lifetime, reliability, resource-leak

Description

Reports a return that hands back the object a using is responsible for disposing. The object is disposed as control leaves the block, so the caller receives it already disposed.

Rationale

using is a promise about a point in time: whatever happens inside the block, the resource is disposed when control leaves it. A return is control leaving it. The compiler dutifully inserts the disposal in the generated finally, which runs after the return value has been computed and before the caller gets it — so the method appears to return a working object and in fact returns a disposed one.

What the caller sees depends on the type. Well-behaved types throw ObjectDisposedException on first use, which at least points somewhere useful. Others silently misbehave: a reader over a closed stream returns nothing, a connection reports itself closed, a cancellation source never fires. In every case the failure surfaces in the caller, far from the method that created the problem, and the method’s signature promises a live object.

Both spellings have the same defect. The block form disposes at the closing brace; the declaration form using var x = …​; disposes at the end of the enclosing block, which a return also crosses.

Returning a value derived from the resource is the correct pattern and is not reported: reading a string out of a reader, copying bytes into an array, or projecting a query into a list all produce values that outlive the resource — which is precisely what using is for.

using System.IO;

public class Loader
{
    public StreamReader Broken(string path)
    {
        using (var reader = new StreamReader(path))
        {
            return reader;                    // FLAW — disposed before the caller sees it
        }
    }

    public string Contents(string path)
    {
        using (var reader = new StreamReader(path))
        {
            return reader.ReadToEnd();        // OK — the string outlives the reader
        }
    }

    public StreamReader Transfer(string path)
    {
        return new StreamReader(path);        // OK — ownership passes to the caller
    }
}

Remediation

Decide who owns the resource.

  • The caller owns it: drop the using and return the object directly. The method now transfers ownership, and its callers become responsible for the disposal — normally by putting the call in a using of their own. Say so in the method’s documentation.

  • The method owns it: keep the using and return a value that does not depend on the resource — the decoded string, the materialised list, the copied buffer.

A wrapper type that owns the inner resource and disposes it in its own Dispose is the third option when the caller genuinely needs streaming access.

References