Skip to content

Commit

Permalink
Merge pull request #371 from IBM-Cloud/dev
Browse files Browse the repository at this point in the history
Fix: Excluding Windows `GOOS` from `flock` file mutex logic
  • Loading branch information
tonystarkjr3 authored Apr 3, 2023
2 parents 9cca421 + 6394821 commit 3802224
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
46 changes: 41 additions & 5 deletions bluemix/configuration/persistor.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"io/ioutil"
"os"
"path/filepath"
"runtime"
"strings"
"time"

"github.com/IBM-Cloud/ibm-cloud-cli-sdk/common/file_helpers"
Expand All @@ -31,28 +33,39 @@ type DiskPersistor struct {
filePath string
fileLock *flock.Flock
parentContext context.Context
runtimeGOOS string
}

func NewDiskPersistor(path string) DiskPersistor {
return DiskPersistor{
filePath: path,
fileLock: flock.New(path),
parentContext: context.Background(),
runtimeGOOS: runtime.GOOS,
}
}

func (dp DiskPersistor) Exists() bool {
return file_helpers.FileExists(dp.filePath)
}

func (dp *DiskPersistor) windowsLockedRead(data DataInterface) error {
// TO DO: exclusive file-locking for the reading NOT yet implemented
return dp.read(data)
}

func isBlockingLockError(err error) bool {
return err != nil && !strings.Contains(err.Error(), "no such file or directory")
}

func (dp *DiskPersistor) lockedRead(data DataInterface) error {
lockCtx, cancelLockCtx := context.WithTimeout(dp.parentContext, 30*time.Second) /* allotting a 30-second timeout means there can be a maximum of 298 failed retrials (each up to 500 ms, as
specified after the deferred call to cancelLockCtx). 30 appears to be a conventional value for a parent context passed to TryLockContext, as per docs */
defer cancelLockCtx()
_, lockErr := dp.fileLock.TryLockContext(lockCtx, 100*time.Millisecond) /* provide a file lock just while dp.read is called, because it calls an unmarshaling function
The boolean (first return value) can be wild-carded because lockErr must be non-nil when the lock-acquiring fails (whereby the boolean will be false) */
defer dp.fileLock.Unlock()
if lockErr != nil {
if isBlockingLockError(lockErr) {
return lockErr
}
readErr := dp.read(data)
Expand All @@ -62,26 +75,49 @@ func (dp *DiskPersistor) lockedRead(data DataInterface) error {
return nil
}

func (dp DiskPersistor) readWithFileLock(data DataInterface) error {
switch dp.runtimeGOOS {
case "windows":
return dp.windowsLockedRead(data)
default:
return dp.lockedRead(data)
}
}

func (dp DiskPersistor) writeWithFileLock(data DataInterface) error {
switch dp.runtimeGOOS {
case "windows":
return dp.windowsLockedWrite(data)
default:
return dp.lockedWrite(data)
}
}

func (dp DiskPersistor) Load(data DataInterface) error {
err := dp.lockedRead(data)
err := dp.readWithFileLock(data)
if os.IsPermission(err) {
return err
}

if err != nil { /* would happen if there was nothing to read (EOF) */
err = dp.lockedWrite(data)
err = dp.writeWithFileLock(data)
}
return err
}

func (dp *DiskPersistor) windowsLockedWrite(data DataInterface) error {
// TO DO: exclusive file-locking for the writing NOT yet implemented
return dp.write(data)
}

func (dp DiskPersistor) lockedWrite(data DataInterface) error {
lockCtx, cancelLockCtx := context.WithTimeout(dp.parentContext, 30*time.Second) /* allotting a 30-second timeout means there can be a maximum of 298 failed retrials (each up to 500 ms, as
specified after the deferred call to cancelLockCtx). 30 appears to be a conventional value for a parent context passed to TryLockContext, as per docs */
defer cancelLockCtx()
_, lockErr := dp.fileLock.TryLockContext(lockCtx, 100*time.Millisecond) /* provide a file lock just while dp.read is called, because it calls an unmarshaling function
The boolean (first return value) can be wild-carded because lockErr must be non-nil when the lock-acquiring fails (whereby the boolean will be false) */
defer dp.fileLock.Unlock()
if lockErr != nil {
if isBlockingLockError(lockErr) {
return lockErr
}
writeErr := dp.write(data)
Expand All @@ -92,7 +128,7 @@ func (dp DiskPersistor) lockedWrite(data DataInterface) error {
}

func (dp DiskPersistor) Save(data DataInterface) error {
return dp.lockedWrite(data)
return dp.writeWithFileLock(data)
}

func (dp DiskPersistor) read(data DataInterface) error {
Expand Down
2 changes: 1 addition & 1 deletion bluemix/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package bluemix
import "fmt"

// Version is the SDK version
var Version = VersionType{Major: 1, Minor: 0, Build: 2}
var Version = VersionType{Major: 1, Minor: 0, Build: 3}

// VersionType describe version info
type VersionType struct {
Expand Down

0 comments on commit 3802224

Please sign in to comment.