From ffd5ea4379c04ef93e1e250a3cf689d4af99bd19 Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" Date: Mon, 22 Jan 2018 15:57:44 -0500 Subject: [PATCH] Add support for shared locks Done by adding *RLock* variants of the Lock* methods and removing the file truncation. The two sets of methods rely the same code by dispatching to shared methods, only varying the lock type. The exception is RLocked(), which checks its own, separate boolean. All the lock operations share the same mutex, though, so any all operations can block each other. The docs have been updated to reflet that fact. Also fixed the Windows Unlock() docs to refer to UnlockFileEx() instead of syscall.LOCK_UN. --- flock.go | 31 ++++++++++--- flock_test.go | 115 +++++++++++++++++++++++++++++++++++++++++++++++ flock_unix.go | 76 ++++++++++++++++++++++--------- flock_winapi.go | 9 ++++ flock_windows.go | 74 +++++++++++++++++++++--------- 5 files changed, 256 insertions(+), 49 deletions(-) diff --git a/flock.go b/flock.go index ac40ec6..29b293f 100644 --- a/flock.go +++ b/flock.go @@ -24,6 +24,7 @@ type Flock struct { m sync.RWMutex fh *os.File l bool + r bool } // NewFlock is a function to return a new instance of *Flock. The only parameter @@ -44,18 +45,37 @@ func (f *Flock) Locked() bool { return f.l } +// RLocked is a function to return the current read lock state (locked: true, unlocked: false). +func (f *Flock) RLocked() bool { + f.m.RLock() + defer f.m.RUnlock() + return f.r +} + func (f *Flock) String() string { return f.path } -// TryLockContext repeatedly tries locking until one of the conditions is met: -// TryLock succeeds, TryLock fails with error, or Context Done channel is closed. +// TryLockContext repeatedly tries to take an exclusive lock until one of the +// conditions is met: TryLock succeeds, TryLock fails with error, or Context +// Done channel is closed. func (f *Flock) TryLockContext(ctx context.Context, retryDelay time.Duration) (bool, error) { + return tryCtx(f.TryLock, ctx, retryDelay) +} + +// TryRLockContext repeatedly tries to take a shared lock until one of the +// conditions is met: TryRLock succeeds, TryRLock fails with error, or Context +// Done channel is closed. +func (f *Flock) TryRLockContext(ctx context.Context, retryDelay time.Duration) (bool, error) { + return tryCtx(f.TryRLock, ctx, retryDelay) +} + +func tryCtx(fn func() (bool, error), ctx context.Context, retryDelay time.Duration) (bool, error) { if ctx.Err() != nil { return false, ctx.Err() } for { - if ok, err := f.TryLock(); ok || err != nil { + if ok, err := fn(); ok || err != nil { return ok, err } select { @@ -69,9 +89,8 @@ func (f *Flock) TryLockContext(ctx context.Context, retryDelay time.Duration) (b func (f *Flock) setFh() error { // open a new os.File instance - // create it if it doesn't exist, truncate it if it does exist, open the file read-write - fh, err := os.OpenFile(f.path, os.O_CREATE|os.O_TRUNC|os.O_RDWR, os.FileMode(0600)) - + // create it if it doesn't exist, and open the file read-only. + fh, err := os.OpenFile(f.path, os.O_CREATE|os.O_RDONLY, os.FileMode(0600)) if err != nil { return err } diff --git a/flock_test.go b/flock_test.go index ab6562c..cef2c04 100644 --- a/flock_test.go +++ b/flock_test.go @@ -50,6 +50,7 @@ func (t *TestSuite) TestNewFlock(c *C) { c.Assert(f, Not(IsNil)) c.Check(f.Path(), Equals, t.path) c.Check(f.Locked(), Equals, false) + c.Check(f.RLocked(), Equals, false) } func (t *TestSuite) TestFlock_Path(c *C) { @@ -64,6 +65,12 @@ func (t *TestSuite) TestFlock_Locked(c *C) { c.Check(locked, Equals, false) } +func (t *TestSuite) TestFlock_RLocked(c *C) { + var locked bool + locked = t.flock.RLocked() + c.Check(locked, Equals, false) +} + func (t *TestSuite) TestFlock_String(c *C) { var str string str = t.flock.String() @@ -72,6 +79,7 @@ func (t *TestSuite) TestFlock_String(c *C) { func (t *TestSuite) TestFlock_TryLock(c *C) { c.Assert(t.flock.Locked(), Equals, false) + c.Assert(t.flock.RLocked(), Equals, false) var locked bool var err error @@ -80,6 +88,7 @@ func (t *TestSuite) TestFlock_TryLock(c *C) { c.Assert(err, IsNil) c.Check(locked, Equals, true) c.Check(t.flock.Locked(), Equals, true) + c.Check(t.flock.RLocked(), Equals, false) locked, err = t.flock.TryLock() c.Assert(err, IsNil) @@ -92,6 +101,39 @@ func (t *TestSuite) TestFlock_TryLock(c *C) { c.Check(locked, Equals, false) } +func (t *TestSuite) TestFlock_TryRLock(c *C) { + c.Assert(t.flock.Locked(), Equals, false) + c.Assert(t.flock.RLocked(), Equals, false) + + var locked bool + var err error + + locked, err = t.flock.TryRLock() + c.Assert(err, IsNil) + c.Check(locked, Equals, true) + c.Check(t.flock.Locked(), Equals, false) + c.Check(t.flock.RLocked(), Equals, true) + + locked, err = t.flock.TryRLock() + c.Assert(err, IsNil) + c.Check(locked, Equals, true) + + // shared lock should not block. + flock2 := flock.NewFlock(t.path) + locked, err = flock2.TryRLock() + c.Assert(err, IsNil) + c.Check(locked, Equals, true) + + // make sure we just return false with no error in cases + // where we would have been blocked + t.flock.Unlock() + flock2.Unlock() + t.flock.Lock() + locked, err = flock.NewFlock(t.path).TryRLock() + c.Assert(err, IsNil) + c.Check(locked, Equals, false) +} + func (t *TestSuite) TestFlock_TryLockContext(c *C) { // happy path ctx, cancel := context.WithCancel(context.Background()) @@ -113,6 +155,29 @@ func (t *TestSuite) TestFlock_TryLockContext(c *C) { c.Check(locked, Equals, false) } +func (t *TestSuite) TestFlock_TryRLockContext(c *C) { + // happy path + ctx, cancel := context.WithCancel(context.Background()) + locked, err := t.flock.TryRLockContext(ctx, time.Second) + c.Assert(err, IsNil) + c.Check(locked, Equals, true) + + // context already canceled + cancel() + locked, err = flock.NewFlock(t.path).TryRLockContext(ctx, time.Second) + c.Assert(err, Equals, context.Canceled) + c.Check(locked, Equals, false) + + // timeout + t.flock.Unlock() + t.flock.Lock() + ctx, cancel = context.WithTimeout(context.Background(), 10*time.Millisecond) + defer cancel() + locked, err = flock.NewFlock(t.path).TryRLockContext(ctx, time.Second) + c.Assert(err, Equals, context.DeadlineExceeded) + c.Check(locked, Equals, false) +} + func (t *TestSuite) TestFlock_Unlock(c *C) { var err error @@ -124,6 +189,7 @@ func (t *TestSuite) TestFlock_Unlock(c *C) { c.Assert(err, IsNil) c.Assert(locked, Equals, true) c.Assert(t.flock.Locked(), Equals, true) + c.Check(t.flock.RLocked(), Equals, false) _, err = os.Stat(t.path) c.Assert(os.IsNotExist(err), Equals, false) @@ -131,16 +197,19 @@ func (t *TestSuite) TestFlock_Unlock(c *C) { err = t.flock.Unlock() c.Assert(err, IsNil) c.Check(t.flock.Locked(), Equals, false) + c.Check(t.flock.RLocked(), Equals, false) } func (t *TestSuite) TestFlock_Lock(c *C) { c.Assert(t.flock.Locked(), Equals, false) + c.Check(t.flock.RLocked(), Equals, false) var err error err = t.flock.Lock() c.Assert(err, IsNil) c.Check(t.flock.Locked(), Equals, true) + c.Check(t.flock.RLocked(), Equals, false) // test that the short-circuit works err = t.flock.Lock() @@ -170,5 +239,51 @@ func (t *TestSuite) TestFlock_Lock(c *C) { c.Assert(ok, Equals, true) c.Assert(errCh, IsNil) c.Check(t.flock.Locked(), Equals, false) + c.Check(t.flock.RLocked(), Equals, false) c.Check(gf.Locked(), Equals, true) + c.Check(gf.RLocked(), Equals, false) +} + +func (t *TestSuite) TestFlock_RLock(c *C) { + c.Assert(t.flock.Locked(), Equals, false) + c.Check(t.flock.RLocked(), Equals, false) + + var err error + + err = t.flock.RLock() + c.Assert(err, IsNil) + c.Check(t.flock.Locked(), Equals, false) + c.Check(t.flock.RLocked(), Equals, true) + + // test that the short-circuit works + err = t.flock.RLock() + c.Assert(err, IsNil) + + // + // Test that RLock() is a blocking call + // + ch := make(chan error, 2) + gf := flock.NewFlock(t.path) + defer gf.Unlock() + + go func(ch chan<- error) { + ch <- nil + ch <- gf.RLock() + close(ch) + }(ch) + + errCh, ok := <-ch + c.Assert(ok, Equals, true) + c.Assert(errCh, IsNil) + + err = t.flock.Unlock() + c.Assert(err, IsNil) + + errCh, ok = <-ch + c.Assert(ok, Equals, true) + c.Assert(errCh, IsNil) + c.Check(t.flock.Locked(), Equals, false) + c.Check(t.flock.RLocked(), Equals, false) + c.Check(gf.Locked(), Equals, false) + c.Check(gf.RLocked(), Equals, true) } diff --git a/flock_unix.go b/flock_unix.go index 8c9a64f..40f14df 100644 --- a/flock_unix.go +++ b/flock_unix.go @@ -10,18 +10,33 @@ import ( "syscall" ) -// Lock is a blocking call to try and take the file lock. It will wait until it -// is able to obtain the exclusive file lock. It's recommended that TryLock() be -// used over this function. This function may block the ability to query the -// current Locked() status due to a RW-mutex lock. +// Lock is a blocking call to try and take an exclusive file lock. It will wait +// until it is able to obtain the exclusive file lock. It's recommended that +// TryLock() be used over this function. This function may block the ability to +// query the current Locked() or RLocked() status due to a RW-mutex lock. // -// If we are already locked, this function short-circuits and returns immediately -// assuming it can take the mutex lock. +// If we are already exclusive-locked, this function short-circuits and returns +// immediately assuming it can take the mutex lock. func (f *Flock) Lock() error { + return f.lock(&f.l, syscall.LOCK_EX) +} + +// RLock is a blocking call to try and take a ahred file lock. It will wait +// until it is able to obtain the shared file lock. It's recommended that +// TryRLock() be used over this function. This function may block the ability to +// query the current Locked() or RLocked() status due to a RW-mutex lock. +// +// If we are already shared-locked, this function short-circuits and returns +// immediately assuming it can take the mutex lock. +func (f *Flock) RLock() error { + return f.lock(&f.r, syscall.LOCK_SH) +} + +func (f *Flock) lock(locked *bool, flag int) error { f.m.Lock() defer f.m.Unlock() - if f.l { + if *locked { return nil } @@ -31,27 +46,27 @@ func (f *Flock) Lock() error { } } - if err := syscall.Flock(int(f.fh.Fd()), syscall.LOCK_EX); err != nil { + if err := syscall.Flock(int(f.fh.Fd()), flag); err != nil { return err } - f.l = true + *locked = true return nil } // Unlock is a function to unlock the file. This file takes a RW-mutex lock, so -// while it is running the Locked() function will be blocked. +// while it is running the Locked() and RLocked() functions will be blocked. // // This function short-circuits if we are unlocked already. If not, it calls -// syscall.LOCK_UN on the file and closes the file descriptor It does not remove -// the file from disk. It's up to your application to do. +// syscall.LOCK_UN on the file and closes the file descriptor. It does not +// remove the file from disk. It's up to your application to do. func (f *Flock) Unlock() error { f.m.Lock() defer f.m.Unlock() // if we aren't locked or if the lockfile instance is nil // just return a nil error because we are unlocked - if !f.l || f.fh == nil { + if (!f.l && !f.r) || f.fh == nil { return nil } @@ -63,24 +78,41 @@ func (f *Flock) Unlock() error { f.fh.Close() f.l = false + f.r = false f.fh = nil return nil } -// TryLock is the preferred function for taking a file lock. This function does -// take a RW-mutex lock before it tries to lock the file, so there is the -// possibility that this function may block for a short time if another goroutine -// is trying to take any action. +// TryLock is the preferred function for taking an exclusive file lock. This +// function takes an RW-mutex lock before it tries to lock the file, so there is +// the possibility that this function may block for a short time if another +// goroutine is trying to take any action. // // The actual file lock is non-blocking. If we are unable to get the exclusive -// file lock, the function will return false instead of waiting for the lock. -// If we get the lock, we also set the *Flock instance as being locked. +// file lock, the function will return false instead of waiting for the lock. If +// we get the lock, we also set the *Flock instance as being exclusive-locked. func (f *Flock) TryLock() (bool, error) { + return f.try(&f.l, syscall.LOCK_EX) +} + +// TryRLock is the preferred function for taking a shared file lock. This +// function takes an RW-mutex lock before it tries to lock the file, so there is +// the possibility that this function may block for a short time if another +// goroutine is trying to take any action. +// +// The actual file lock is non-blocking. If we are unable to get the shared file +// lock, the function will return false instead of waiting for the lock. If we +// get the lock, we also set the *Flock instance as being share-locked. +func (f *Flock) TryRLock() (bool, error) { + return f.try(&f.r, syscall.LOCK_SH) +} + +func (f *Flock) try(locked *bool, flag int) (bool, error) { f.m.Lock() defer f.m.Unlock() - if f.l { + if *locked { return true, nil } @@ -90,13 +122,13 @@ func (f *Flock) TryLock() (bool, error) { } } - err := syscall.Flock(int(f.fh.Fd()), syscall.LOCK_EX|syscall.LOCK_NB) + err := syscall.Flock(int(f.fh.Fd()), flag|syscall.LOCK_NB) switch err { case syscall.EWOULDBLOCK: return false, nil case nil: - f.l = true + *locked = true return true, nil } diff --git a/flock_winapi.go b/flock_winapi.go index 683c61e..fe405a2 100644 --- a/flock_winapi.go +++ b/flock_winapi.go @@ -20,8 +20,17 @@ var ( const ( winLockfileFailImmediately = 0x00000001 winLockfileExclusiveLock = 0x00000002 + winLockfileSharedLock = 0x00000000 ) +// Use of 0x00000000 for the shared lock is a guess based on some the MS Windows +// `LockFileEX` docs, which document the `LOCKFILE_EXCLUSIVE_LOCK` flag as: +// +// > The function requests an exclusive lock. Otherwise, it requests a shared +// > lock. +// +// https://msdn.microsoft.com/en-us/library/windows/desktop/aa365203(v=vs.85).aspx + func lockFileEx(handle syscall.Handle, flags uint32, reserved uint32, numberOfBytesToLockLow uint32, numberOfBytesToLockHigh uint32, offset *syscall.Overlapped) (bool, syscall.Errno) { r1, _, errNo := syscall.Syscall6( uintptr(procLockFileEx), diff --git a/flock_windows.go b/flock_windows.go index a071eb2..a0103f6 100644 --- a/flock_windows.go +++ b/flock_windows.go @@ -12,18 +12,33 @@ import ( // lock would block and you ask to fail immediately. const ErrorLockViolation syscall.Errno = 0x21 // 33 -// Lock is a blocking call to try and take the file lock. It will wait until it -// is able to obtain the exclusive file lock. It's recommended that TryLock() be -// used over this function. This function may block the ability to query the -// current Locked() status due to a RW-mutex lock. +// Lock is a blocking call to try and take an exclusive file lock. It will wait +// until it is able to obtain the exclusive file lock. It's recommended that +// TryLock() be used over this function. This function may block the ability to +// query the current Locked() or RLocked() status due to a RW-mutex lock. // -// If we are already locked, this function short-circuits and returns immediately -// assuming it can take the mutex lock. +// If we are already locked, this function short-circuits and returns +// immediately assuming it can take the mutex lock. func (f *Flock) Lock() error { + return f.lock(&f.l, winLockfileExclusiveLock) +} + +// RLock is a blocking call to try and take a sahred file lock. It will wait +// until it is able to obtain the shared file lock. It's recommended that +// TryRLock() be used over this function. This function may block the ability to +// query the current Locked() or RLocked() status due to a RW-mutex lock. +// +// If we are already locked, this function short-circuits and returns +// immediately assuming it can take the mutex lock. +func (f *Flock) RLock() error { + return f.lock(&f.r, winLockfileSharedLock) +} + +func (f *Flock) lock(locked *bool, flag uint32) error { f.m.Lock() defer f.m.Unlock() - if f.l { + if *locked { return nil } @@ -33,19 +48,19 @@ func (f *Flock) Lock() error { } } - if _, errNo := lockFileEx(syscall.Handle(f.fh.Fd()), winLockfileExclusiveLock, 0, 1, 0, &syscall.Overlapped{}); errNo > 0 { + if _, errNo := lockFileEx(syscall.Handle(f.fh.Fd()), flag, 0, 1, 0, &syscall.Overlapped{}); errNo > 0 { return errNo } - f.l = true + *locked = true return nil } // Unlock is a function to unlock the file. This file takes a RW-mutex lock, so -// while it is running the Locked() function will be blocked. +// while it is running the Locked() and RLocked() functions will be blocked. // // This function short-circuits if we are unlocked already. If not, it calls -// syscall.LOCK_UN on the file and closes the file descriptor It does not remove +// UnlockFileEx() on the file and closes the file descriptor. It does not remove // the file from disk. It's up to your application to do. func (f *Flock) Unlock() error { f.m.Lock() @@ -53,7 +68,7 @@ func (f *Flock) Unlock() error { // if we aren't locked or if the lockfile instance is nil // just return a nil error because we are unlocked - if !f.l || f.fh == nil { + if (!f.l && !f.r) || f.fh == nil { return nil } @@ -65,24 +80,41 @@ func (f *Flock) Unlock() error { f.fh.Close() f.l = false + f.r = false f.fh = nil return nil } -// TryLock is the preferred function for taking a file lock. This function does -// take a RW-mutex lock before it tries to lock the file, so there is the -// possibility that this function may block for a short time if another goroutine -// is trying to take any action. +// TryLock is the preferred function for taking an exlusive file lock. This +// function does take a RW-mutex lock before it tries to lock the file, so there +// is the possibility that this function may block for a short time if another +// goroutine is trying to take any action. // // The actual file lock is non-blocking. If we are unable to get the exclusive -// file lock, the function will return false instead of waiting for the lock. -// If we get the lock, we also set the *Flock instance as being locked. +// file lock, the function will return false instead of waiting for the lock. If +// we get the lock, we also set the *Flock instance as being exclusive-locked. func (f *Flock) TryLock() (bool, error) { + return f.try(&f.l, winLockfileExclusiveLock) +} + +// TryRLock is the preferred function for taking a shared file lock. This +// function does take a RW-mutex lock before it tries to lock the file, so there +// is the possibility that this function may block for a short time if another +// goroutine is trying to take any action. +// +// The actual file lock is non-blocking. If we are unable to get the shared file +// lock, the function will return false instead of waiting for the lock. If we +// get the lock, we also set the *Flock instance as being shared-locked. +func (f *Flock) TryRLock() (bool, error) { + return f.try(&f.r, winLockfileSharedLock) +} + +func (f *Flock) try(locked *bool, flag uint32) (bool, error) { f.m.Lock() defer f.m.Unlock() - if f.l { + if *locked { return true, nil } @@ -92,7 +124,7 @@ func (f *Flock) TryLock() (bool, error) { } } - _, errNo := lockFileEx(syscall.Handle(f.fh.Fd()), winLockfileExclusiveLock|winLockfileFailImmediately, 0, 1, 0, &syscall.Overlapped{}) + _, errNo := lockFileEx(syscall.Handle(f.fh.Fd()), flag|winLockfileFailImmediately, 0, 1, 0, &syscall.Overlapped{}) if errNo > 0 { if errNo == ErrorLockViolation || errNo == syscall.ERROR_IO_PENDING { @@ -102,7 +134,7 @@ func (f *Flock) TryLock() (bool, error) { return false, errNo } - f.l = true + *locked = true return true, nil }