Skip to content
This repository has been archived by the owner on Mar 9, 2019. It is now read-only.

Cleanup file descriptors on failed database initialization #725

Closed
wants to merge 1 commit into from

Conversation

cj-dimaggio
Copy link

@cj-dimaggio cj-dimaggio commented Aug 29, 2017

Came across this one today on a Unix system...

So when you try to open up a Bolt db that doesn't exist already exist in ReadOnly mode you get an os.PathError when initializing your meta pages and Bolt returns nil and the error. Which makes sense; you've opened (and created) the file with the O_RDONLY flag and are now trying to write to it. I don't, personally, think that in ReadOnly mode you should initialize the database.

However the issue is that this happens after you've acquired your shared lock on the FD and you now have no (direct) means of releasing it (because you've been passed back only nil). So if in my program I want to handle this error by creating and initializing a new database in ReadWrite mode I can't because I hang indefinitely waiting for my exclusive lock.
All that this change does is make sure we call close on our partially created database before returning, closing our file so that a subsequent call to Open has a chance of acquiring an exclusive lock on the file.

Fixes an issue where failing to open a non-existent database in ReadOnly
mode would make you unable to properly initialize it in ReadWrite mode
afterwards due to a hanging lock.
Copy link

@heyitsanthony heyitsanthony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs _ = db.close() to satisfy errcheck

@@ -193,10 +193,12 @@ func Open(path string, mode os.FileMode, options *Options) (*DB, error) {

// Initialize the database if it doesn't exist.
if info, err := db.file.Stat(); err != nil {
db.close()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ = db.close()

return nil, err
} else if info.Size() == 0 {
// Initialize new files with meta pages.
if err := db.init(); err != nil {
db.close()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ = db.close()

@heyitsanthony
Copy link

@Ssawa this patch probably won't get merged here, but I'll be happy to add it to http://github.com/coreos/bbolt

@vincent-petithory
Copy link
Contributor

vincent-petithory commented Aug 30, 2017 via email

@heyitsanthony
Copy link

@vincent-petithory my apologies for trying to maintain this software

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants