Proper way to release resources with defer in a loop?

Question

I need to make SQL queries to database in the loop:

for rows.Next() {

   fields, err := db.Query(.....)
   if err != nil {
      // ...
   }
   defer fields.Close()

   // do something with `fields`

}

What will be better: leave all as is or move defer after loop:

for rows.Next() {

   fields, err := db.Query(.....)
   if err != nil {
      // ...
   }

   // do something with `fields`
}

defer fields.Close()

Or something else ?

Poor

Answer 1

The whole point of defer is that it does not execute until the function returns, so the appropriate place to put it would be immediately after the resource you want to close is opened. However, since you're creating the resource inside the loop, you should not use defer at all - otherwise, you're not going to close any of the resources created inside the loop until the function exits, so they'll pile up until then. Instead, you should close them at the end of each loop iteration, without defer:

for rows.Next() {

   fields, err := db.Query(.....)
   if err != nil {
      // ...
   }

   // do something with `fields`

   fields.Close()
}

Collazo

Adding to this, in this case the defer won't even work as the OP expects, as it will only close the last fields from the loop (it needs a closure to work correctly). Wrapping the loop inner body in an anonymous func with a defer might be a good solution, btw. – Commuter

True - but even with the closure to work correctly, it will still not work well. – Collazo

It's the other way. If you use closure for defer only the last one will be called. For defer fields.Close() each call will correctly point to different pointer, of course, it's still wrong as all will be called once the func finishes. – Ovule

Does it mean that if you allocate multiple resources within each iteration of a loop, and error happens, and you panic inside the if clause without closing each opened resource first, resources allocated during last iteration won't be properly closed? I.e. in for loops one can not rely on automatic resource clean up and has to manually clean all resources allocated within this loop iteration in case of error? – Chemush

If you don't defer closure and you recover the panic without closing the resources in the recovery, yes, you may leak resources, regardless of everything else. Panics should be rare and generally should be fatal. If you're going to recover panics, you should be keenly aware of the repercussions. – Collazo

Answer 2

Execution of a deferred function is not only delayed, deferred to the moment the surrounding function returns, it is also executed even if the enclosing function terminates abruptly, e.g. panics. Spec: Defer statements:

A "defer" statement invokes a function whose execution is deferred to the moment the surrounding function returns, either because the surrounding function executed a return statement, reached the end of its function body, or because the corresponding goroutine is panicking.

Whenever you create a value or a resource that provides means to properly close it / dispose of it, you should always use a defer statement to make sure it is released even if your other code panics to prevent leaking memory or other system resources.

It's true that if you're allocating resources in a loop you should not simply use defer, as then releasing resources will not happen as early as it could and should (at the end of each iteration), only after the for statement (only after all iterations).

What you should do is that if you have a snippet that allocates such resources, wrap it in a function –either an anonymous or a named function–, and in that function you may use defer, and resources will be freed as soon as they are no longer needed, and what's important is that even if there is a bug in your code which may panic.

Example:

for rows.Next() {
    func() {
        fields, err := db.Query(...)
        if err != nil {
            // Handle error and return
            return
        }
        defer fields.Close()

        // do something with `fields`
    }()
}

Or if put in a named function:

func foo(rs *db.Rows) {
    fields, err := db.Query(...)
    if err != nil {
        // Handle error and return
        return
    }
    defer fields.Close()

    // do something with `fields`
}

And calling it:

for rows.Next() {
    foo(rs)
}

Also if you'd want to terminate on the first error, you could return the error from foo():

func foo(rs *db.Rows) error {
    fields, err := db.Query(...)
    if err != nil {
        return fmt.Errorf("db.Query error: %w", err)
    }
    defer fields.Close()

    // do something with `fields`
    return nil
}

And calling it:

for rows.Next() {
    if err := foo(rs); err != nil {
        // Handle error and return
        return
    }
}

Also note that Rows.Close() returns an error which when called using defer is discarded. If we want to check the returned error, we can use an anonymous function like this:

func foo(rs *db.Rows) (err error) {
    fields, err := db.Query(...)
    if err != nil {
        return fmt.Errorf("db.Query error: %w", err)
    }
    defer func() {
        if err = fields.Close(); err != nil {
            err = fmt.Errorf("Rows.Close() error: %w", err)
        }
    }()

    // do something with `fields`
    return nil
}

Gorlin

You can see an example of using a function wrapper vs a vanilla loop here: go.dev/play/p/Ma498SWhr7I – Torrefy

What if the resource that need to closed is the loop variable itself like shown here go.dev/play/p/6j8XIui7ACz – Swearingen

@RamDittakavi Same answer: use an anonymous or named function inside the loop body, and inside that, use defer. It will be executed before the next iteration (and with that overwrite of the loop variable) happens. – Gorlin

Excellent answer. One note on the last Rows.Close() catching error: foo should return the error that happened on db.Query AND/OR fields.Close(), so either format more context into error using fmt.Errorf("%w\nRows.Close() error: %v", err, err2) if both happened, return just the one that happened, or join the errors with the err = errors.Join(err, err2) func if your version of Go has it, which will ignore nil errors. – Immutable

Answer 3

You can construct a local function to solve this problem

    for i := 0; i < 5; i++ {
        func() {
            f, err := os.Open("/path/to/file")
            if err != nil {
                log.Fatal(err)
            } else {
                defer f.Close()
            }
        }()
    }

Pneumo

Shouldn't the defer be before error checking? – Latoyialatreece

No. If an error occurred while opening the file, and the file does not need to be closed. – Pneumo

I think the else is not needed, you can remove it. – Mantel