Skip to content

Commit

Permalink
Fix Windows flock/funlock race (#122)
Browse files Browse the repository at this point in the history
Signed-off-by: John Howard <jhoward@microsoft.com>
  • Loading branch information
John Howard authored and xiang90 committed Sep 12, 2018
1 parent 8987c97 commit 7ee3ded
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 26 deletions.
3 changes: 1 addition & 2 deletions bolt_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 1 addition & 2 deletions bolt_unix_solaris.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package bbolt

import (
"fmt"
"os"
"syscall"
"time"
"unsafe"
Expand All @@ -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()
Expand Down
32 changes: 14 additions & 18 deletions bolt_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
6 changes: 2 additions & 4 deletions db.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
28 changes: 28 additions & 0 deletions db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 7ee3ded

Please sign in to comment.