G307: Why is defer os.File.Close bad?

[imirkin]

It seems like this was recently added, but without any justification. If a file close fails, there's no real recourse to that... what is one supposed to do with it?

I would actually imagine that defer foo() where foo returns an error is the bad case, except os.File.Close, which is clearly OK...

[ccojocar]

Please see for more details this article https://www.joeshaw.org/dont-defer-close-on-writable-files/ and also the 3rd point on this https://blog.learngoprogramming.com/5-gotchas-of-defer-in-go-golang-part-iii-36a1ab3d6ef1.

[ccojocar]

Thanks for reporting this. I am going to close it for now.

[sjl]

The first update in the first blog post mentions that a common solution to this problem is to defer one Close() operation on the file to ensure it gets closed no matter what, as long as you also check an explicit Close() call later:

func doSomething() error {
    f, err := os.Create("foo")
    if err != nil {
        return err
    }
    defer f.Close()

    if _, err := f.Write([]byte("bar"); err != nil {
        return err
    }

    if err := f.Close(); err != nil {
        return err
    }
    return nil
}

The blog post seems mostly okay with this, except that os.File.Close() wasn't explicitly documented to be safe to call Close() on multiple times. Since then, the documentation has been updated. So now it seems that this pattern is a reasonable way to work.

gosec still flags it as a problem, and we have to // #nosec the defer calls all the time, which is annoying. I don't know if it's possible to detect whether .Close() is called later in the function to avoid these false positives, but if it's possible, that would be nice.