diff --git a/bolt_unix.go b/bolt_unix.go index 38e11d4c4..5f2bb5145 100644 --- a/bolt_unix.go +++ b/bolt_unix.go @@ -4,14 +4,13 @@ package bbolt import ( "fmt" - "os" "syscall" "time" "unsafe" ) // flock acquires an advisory lock on a file descriptor. -func flock(db *DB, mode os.FileMode, exclusive bool, timeout time.Duration) error { +func flock(db *DB, exclusive bool, timeout time.Duration) error { var t time.Time if timeout != 0 { t = time.Now() diff --git a/bolt_unix_solaris.go b/bolt_unix_solaris.go index 492eaf302..babad6578 100644 --- a/bolt_unix_solaris.go +++ b/bolt_unix_solaris.go @@ -2,7 +2,6 @@ package bbolt import ( "fmt" - "os" "syscall" "time" "unsafe" @@ -11,7 +10,7 @@ import ( ) // flock acquires an advisory lock on a file descriptor. -func flock(db *DB, mode os.FileMode, exclusive bool, timeout time.Duration) error { +func flock(db *DB, exclusive bool, timeout time.Duration) error { var t time.Time if timeout != 0 { t = time.Now() diff --git a/bolt_windows.go b/bolt_windows.go index 4e3f90fb4..fca178bd2 100644 --- a/bolt_windows.go +++ b/bolt_windows.go @@ -16,8 +16,6 @@ var ( ) const ( - lockExt = ".lock" - // see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365203(v=vs.85).aspx flagLockExclusive = 2 flagLockFailImmediately = 1 @@ -48,28 +46,24 @@ func fdatasync(db *DB) error { } // flock acquires an advisory lock on a file descriptor. -func flock(db *DB, mode os.FileMode, exclusive bool, timeout time.Duration) error { - // Create a separate lock file on windows because a process - // cannot share an exclusive lock on the same file. This is - // needed during Tx.WriteTo(). - f, err := os.OpenFile(db.path+lockExt, os.O_CREATE, mode) - if err != nil { - return err - } - db.lockfile = f - +func flock(db *DB, exclusive bool, timeout time.Duration) error { var t time.Time if timeout != 0 { t = time.Now() } - fd := f.Fd() var flag uint32 = flagLockFailImmediately if exclusive { flag |= flagLockExclusive } for { - // Attempt to obtain an exclusive lock. - err := lockFileEx(syscall.Handle(fd), flag, 0, 1, 0, &syscall.Overlapped{}) + // Fix for https://github.com/etcd-io/bbolt/issues/121. Use byte-range + // -1..0 as the lock on the database file. + var m1 uint32 = (1 << 32) - 1 // -1 in a uint32 + err := lockFileEx(syscall.Handle(db.file.Fd()), flag, 0, 1, 0, &syscall.Overlapped{ + Offset: m1, + OffsetHigh: m1, + }) + if err == nil { return nil } else if err != errLockViolation { @@ -88,9 +82,11 @@ func flock(db *DB, mode os.FileMode, exclusive bool, timeout time.Duration) erro // funlock releases an advisory lock on a file descriptor. func funlock(db *DB) error { - err := unlockFileEx(syscall.Handle(db.lockfile.Fd()), 0, 1, 0, &syscall.Overlapped{}) - db.lockfile.Close() - os.Remove(db.path + lockExt) + var m1 uint32 = (1 << 32) - 1 // -1 in a uint32 + err := unlockFileEx(syscall.Handle(db.file.Fd()), 0, 1, 0, &syscall.Overlapped{ + Offset: m1, + OffsetHigh: m1, + }) return err } diff --git a/db.go b/db.go index 4e38ab8c6..d91dcf813 100644 --- a/db.go +++ b/db.go @@ -105,8 +105,7 @@ type DB struct { path string file *os.File - lockfile *os.File // windows only - dataref []byte // mmap'ed readonly, write throws SEGV + dataref []byte // mmap'ed readonly, write throws SEGV data *[maxMapSize]byte datasz int filesz int // current on disk file size @@ -197,8 +196,7 @@ func Open(path string, mode os.FileMode, options *Options) (*DB, error) { // if !options.ReadOnly. // The database file is locked using the shared lock (more than one process may // hold a lock at the same time) otherwise (options.ReadOnly is set). - if err := flock(db, mode, !db.readOnly, options.Timeout); err != nil { - db.lockfile = nil // make 'unused' happy. TODO: rework locks + if err := flock(db, !db.readOnly, options.Timeout); err != nil { _ = db.close() return nil, err } diff --git a/db_test.go b/db_test.go index aa0be75ab..5205bac2b 100644 --- a/db_test.go +++ b/db_test.go @@ -63,6 +63,34 @@ func TestOpen(t *testing.T) { } } +// Regression validation for https://github.com/etcd-io/bbolt/pull/122. +// Tests multiple goroutines simultaneously opening a database. +func TestOpen_MultipleGoroutines(t *testing.T) { + const ( + instances = 30 + iterations = 30 + ) + path := tempfile() + defer os.RemoveAll(path) + var wg sync.WaitGroup + for iteration := 0; iteration < iterations; iteration++ { + for instance := 0; instance < instances; instance++ { + wg.Add(1) + go func() { + defer wg.Done() + db, err := bolt.Open(path, 0600, nil) + if err != nil { + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + }() + } + wg.Wait() + } +} + // Ensure that opening a database with a blank path returns an error. func TestOpen_ErrPathRequired(t *testing.T) { _, err := bolt.Open("", 0666, nil)