Deferred Close Before Error

ID

go.deferred_close_before_error

Severity

high

Remediation Complexity

simple

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

Go

Tags

CWE:476, error-handling, reliability

Description

Reports a resource whose Close() is deferred immediately after a constructor binds it together with an error, before that error has been inspected. When a function returns (value, error) and the deferred close runs on the value while the error is still unexamined, a failed construction leaves the value nil or zero and the deferred Close() dereferences a nil receiver at function return.

Rationale

The Go idiom is to inspect the error and bail out before using or deferring anything on the returned value. A defer value.Close() placed on the statement that immediately follows the binding postpones a nil-receiver dereference to the moment the function returns, where it panics far from the constructor that actually failed. The rule fires only when the binding keeps the error in a real variable (a deliberate _ discard is excluded), the right-hand side is a single call, the error-typed value resolves to error, and the very next statement is defer R.Close() on the bound value (or a field reached through it). A binding followed by an if err != nil guard before the defer is therefore not flagged, because the defer is no longer adjacent.

func fire() {
    rc, err := newReader()
    defer rc.Close() // FLAW — err not checked; a failed open leaves rc nil
    _ = err
}

func ok() error {
    rc, err := newReader()
    if err != nil {
        return err
    }
    defer rc.Close() // OK — error checked before the defer
    return nil
}

Remediation

Check the returned error and return (or otherwise handle the failure) before deferring Close() on the resource, so the deferred call only ever runs on a successfully constructed value.