object_db: prevent FD leaks when writing blobs

[bk2204]

When writing a blob, there are two places where we use temporary files. First, we use them when we write the blob itself. Second, we use them when we encode the buffer. In both of these cases, however, we failed to close the file after we were done with it, leading to file descriptor leaks until Go garbage-collected the items.

Close each of these files when we're done with them. Place the defer statements after the os.Remove statements, since defer statements are done in LIFO order and Windows will be happier if we don't try to remove a file we have open.

With this change, I don't see any of the lingering files I reported in the original issue; they're all routinely closed, and only the pack remains open.

Fixes git-lfs/git-lfs#3689
/cc @ttaylorr as contributor
/cc @larsxschneider as reporter

[ttaylorr]

Thanks for finding the root-cause of this issue. I completely agree with your analysis and believe that
the change here will fix the issue.

I left one comment for your consideration before approving this, but let me know what you think.

[ttaylorr]

This certainly seems correct to me, although I think that grokking it requires that the reader know that defer statements are executed in LIFO order (as you pointed out in your commit message). One way to potentially make this clearer is:

defer func() {
        os.Remove(buf.Name())
        buf.Close()
}()

Which makes it clearer to see that the handle is removed and then the buffer is closed. But, I think that we would take care of both of the call-sites below by writing something like:

func (o *ObjectDatabase) cleanup(f *os.File) {
        os.Remove(f.Name())
        f.Close()
}

and then writing:

defer o.cleanup(buf) // <- on L162
defer o.cleanup(tmp) // <- on L241

Of course, we could go even further and wrap the result of io/ioutil.TempFile in our own type which handles the cleanup for us by implementing io.ReadCloser, but I think that may be going overkill for what we need to do here.

[bk2204]

Sure, I think that would be a good improvement.