defer in a loop

[Matt Kane's Brain]

Say I have a loop that is something like this:

for _, name := range filenames {
f, err := os.Open(name)
if err != nil { panic(err) }
defer f.Close()
// do some stuff
}

If the list of filenames is really big, I'll end up with a whole bunch
of open files until the function that contains the loop returns. (and
if it's a goroutine it might never return!) But I like to think that
the habit of using defer to close resources near where they are
acquired is a good one. Is it overkill to do something like this?

for _, name := range filenames {
func() {
f, err := os.Open(name)
if err != nil { panic(err) }
defer f.Close()
// do some stuff
}()
}

Or is defer not the right thing to do in a case like this?

--
matt kane's brain
http://hydrogenproject.com

[Patrick Mylund Nielsen]

IMO calling f.Close immediately after doing the stuff is preferrable
in this case, and any other case where it is likely that a significant
amount of time will pass between the work with the file is completed,
and the function returns.

[Vanja Pejovic]

Why not make it a named function instead of a closure? I don't think it's overkill.

[sdeg...@8thlight.com]

Often it turns out that "do some stuff" is a significant (not to be confused with large) task and can be its own function, which the loop just calls. A bonus in that case is that you can use defer as normal.

-Steven

[Anthony Martin]

Vanja Pejovic <vva...@gmail.com> once said:
> Why not make it a named function instead of a closure? I don't think it's
> overkill.

There's no runtime overhead for closures that
are called in place or for function literals
that don't close over any variables. They both
behave exactly like any other named function.

Anthony

[Vanja Pejovic]

Oh, that's good to know. Thanks.

[André Moraes]

>
> Or is defer not the right thing to do in a case like this?

Defer is the only guarantee that you can have if something wrong
happens, so use it.

By letting the file.Close() for a later place in the code you add a
possibility that on a later change in your code the Close() call isn't
executed.

In your case, since you had a lot of open files, you should start a
goroutine with your file name and open the file there.

package main

import "sync"

func handleFile(name string, wg *sync.WaitGroup ) {
// make sure to signal when everything is done
defer wg.Done()

f, err := os.Open(name)

if err != nil {

return
}

// use f
defer f.Close()
}

func doLotsOfWork() {
done := &sync.WaitGroup{}
fileCount := 0

for _, name := range filenames {

done.Add(1)
go handleFile(name,done)
}

// block until everything is done
done.Wait()
}

>
> --
> matt kane's brain
> http://hydrogenproject.com

--
André Moraes
http://andredevchannel.blogspot.com/

[André Moraes]

>
>        // use f
>        defer f.Close()

oops, the right way is:

defer f.Close()
// use f

[Ugorji Nwoke]

IMO, I think a goroutine may be heavy handed here. If the OP just wants to ensure a file is eventually closed, and wants it to happen within each iteration of the loop, then the content of each loop should be run in a function (externally defined or closure as he has it) and use defer as usual.

[Matt Kane's Brain]

Actually in my case a goroutine would be terrible as I need to read
the contents of each file in the specified order! But moving it to a
named function is probably reasonable.

The real useful part though is:

> There's no runtime overhead for closures that
> are called in place or for function literals
> that don't close over any variables

On Mon, Apr 23, 2012 at 12:12, Ugorji Nwoke <ugo...@gmail.com> wrote:
> IMO, I think a goroutine may be heavy handed here. If the OP just wants to
> ensure a file is eventually closed, and wants it to happen within each
> iteration of the loop, then the content of each loop should be run in a
> function (externally defined or closure as he has it) and use defer as
> usual.

--

[Rémy Oudompheng]

Le 23 avril 2012 15:45, Matt Kane's Brain
<mkb-...@hydrogenproject.com> a écrit :

> Say I have a loop that is something like this:
>
> for _, name := range filenames {
>    f, err := os.Open(name)
>    if err != nil { panic(err) }
>    defer f.Close()
>    // do some stuff
> }
>
> If the list of filenames is really big, I'll end up with a whole bunch
> of open files until the function that contains the loop returns.

I think you can write:

for _, name := range filenames {
f, err := os.Open(name)
if err != nil { panic(err) }
defer f.Close()
// do some stuff

f.Close()
}

Rémy.

[Paul Ruane]

On Monday, April 23, 2012 2:45:38 PM UTC+1, mkb wrote:

Say I have a loop that is something like this:

for _, name := range filenames {
    f, err := os.Open(name)
    if err != nil { panic(err) }
    defer f.Close()
    // do some stuff
}

I was going to suggest having an extra scoping block so that the defer runs at the end of that instead of the end of the loop:

    for _, name := range filenames {

        {

            f, err := os.Open(name)

            if err != nil { panic(err) }

            defer f.Close()

            // do some stuff

        }

        // I expected the defer to run here

    }

    // the defer actually runs here

But this doesn't work. Anyone know why defer does not run at the end of the scoping block? Is this by design?

[Dave Cheney]

defer statements are evaluated just before the function returns to the caller.

http://golang.org/ref/spec#Defer_statements

Cheers

Dave

[Julien Laffaye]

I've come to appreciate how it forces me to write more named functions,
with fewer lines of code in each one.
In fact it forces me to layer the code!

[Paul Ruane]

Thanks, I'll have to revisit my code -- I fear I have a number of defers running later than I thought.

On Tuesday, April 24, 2012 9:50:07 AM UTC+1, Dave Cheney wrote:

defer statements are evaluated just before the function returns to the caller.

http://golang.org/ref/spec#Defer_statements

Cheers

Dave

[Rémy Oudompheng]

defer runs at the end of the function by design. How would you defer
something under a condition?

If you need to do something at the end of a block, do it at the end of
the block. If you need to collect dangling resources on a panic, use
defer.

If you want both, use both. If you want the action to be done once,
use sync.Once.

Rémy.

[Unknown]

From my experience its safe to call Close multiple times, I use that
pattern all the time to make sure that database connections get closed
asap (and to make sure that if I somehow miss closing them, they get
closed at the end of the function)

[roger peppe]

it's a pity we can't close a channel twice.

[Unknown]

Well, technically you can, you just won't like the result of doing so :D

[Steven Blenkinsop]

On Tue, Apr 24, 2012 at 10:07 AM, Unknown <plague...@gmail.com> wrote:

From my experience its safe to call Close multiple times, I use that
pattern all the time to make sure that database connections get closed
asap (and to make sure that if I somehow miss closing them, they get
closed at the end of the function)

This'll break as soon as you have a "continue"  statement somewhere in your loop. It would probably be best just to use a function to explicitly scope the defer.