Skip to content

Commit

Permalink
cow: Make sure to close files after use (#233)
Browse files Browse the repository at this point in the history
  • Loading branch information
dergoegge authored Dec 3, 2020
1 parent 87746e9 commit 1a03a98
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions accumulator/forestdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ func (m *manifest) commit(basePath string) error {

// Create new manifest on disk
fNewManifest, err := os.OpenFile(fPath, os.O_CREATE|os.O_RDWR, 0666)
defer fNewManifest.Close()
if err != nil {
return err
}
Expand Down Expand Up @@ -279,6 +280,7 @@ func (m *manifest) commit(basePath string) error {
// Overwrite the current manifest number in CURRENT
curFileName := filepath.Join(basePath, "CURRENT")
fCurrent, err := os.OpenFile(curFileName, os.O_CREATE|os.O_WRONLY, 0666)
defer fCurrent.Close()
if err != nil {
return err
}
Expand Down Expand Up @@ -307,6 +309,7 @@ func (m *manifest) load(path string) error {
curFileName := filepath.Join(path, "CURRENT")

curFile, err := os.OpenFile(curFileName, os.O_RDONLY, 0666)
defer curFile.Close()
if err != nil {
return err
}
Expand All @@ -331,6 +334,7 @@ func (m *manifest) load(path string) error {
}

maniFile, err := os.Open(maniFilePath)
defer maniFile.Close()
if err != nil {
return err
}
Expand Down Expand Up @@ -977,6 +981,7 @@ func (cow *cowForest) load(fileNum uint64) error {
fmt.Println("FILE LOADED: ", fName)
}
f, err := os.Open(fName)
defer f.Close()
if err != nil {
// If the error returned is of no files existing, then the manifest
// is corrupt
Expand Down Expand Up @@ -1083,6 +1088,8 @@ func (cow *cowForest) commit() error {
if err != nil {
return err
}

f.Close()

This comment has been minimized.

Copy link
@ysangkok

ysangkok Dec 4, 2020

Contributor

@dergoegge why are you not using defer here? looks like the handle is leaking unclosed in the two error returns on line 1089 and 1085

This comment has been minimized.

Copy link
@dergoegge

dergoegge Dec 5, 2020

Author Contributor

Because of the loop over the cow.cachedTreeTables map. There could be hundreds (configurable by the user) of entries in that map so not closing the file at the end of the loop could still cause a bunch of files to be open when they don't have to be. I did not think about error handling between Open and Close, good catch! 1085 is not a problem (afaik) because if there is a error while opening the file it does not have to be closed. 1089 could cause problems, although probably not frequently.

}

err := cow.manifest.commit(cow.meta.fBasePath)
Expand Down

0 comments on commit 1a03a98

Please sign in to comment.