From 54280520043e175bb221a99f79bb2a51d32ee998 Mon Sep 17 00:00:00 2001 From: Anupam Pokharel <120498245+tonystarkjr3@users.noreply.github.com> Date: Mon, 3 Apr 2023 15:12:52 -0500 Subject: [PATCH 1/2] Fix: Excluding Windows `GOOS` from `flock` file mutex logic (#369) * excluding windows from flock * excluding windows from flock and removing test configs * persistor cleaning * persistor typo fix * porting hotfix for go test suite * commenting better * changing params of NewDiskPersistor --------- Co-authored-by: Anupam Pokharel --- bluemix/configuration/persistor.go | 46 ++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/bluemix/configuration/persistor.go b/bluemix/configuration/persistor.go index a62f17e..80f538f 100644 --- a/bluemix/configuration/persistor.go +++ b/bluemix/configuration/persistor.go @@ -5,6 +5,8 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" + "strings" "time" "github.com/IBM-Cloud/ibm-cloud-cli-sdk/common/file_helpers" @@ -31,6 +33,7 @@ type DiskPersistor struct { filePath string fileLock *flock.Flock parentContext context.Context + runtimeGOOS string } func NewDiskPersistor(path string) DiskPersistor { @@ -38,6 +41,7 @@ func NewDiskPersistor(path string) DiskPersistor { filePath: path, fileLock: flock.New(path), parentContext: context.Background(), + runtimeGOOS: runtime.GOOS, } } @@ -45,6 +49,15 @@ 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 */ @@ -52,7 +65,7 @@ func (dp *DiskPersistor) lockedRead(data DataInterface) error { _, 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) @@ -62,18 +75,41 @@ 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 */ @@ -81,7 +117,7 @@ func (dp DiskPersistor) lockedWrite(data DataInterface) error { _, 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) @@ -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 { From 2800649382920ad825bbb68667f49565791c42b1 Mon Sep 17 00:00:00 2001 From: Anupam Pokharel Date: Mon, 3 Apr 2023 15:17:20 -0500 Subject: [PATCH 2/2] chore: bumping the minor version from 1.0.2 to 1.0.3 --- bluemix/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bluemix/version.go b/bluemix/version.go index fffb171..a3c1ccd 100644 --- a/bluemix/version.go +++ b/bluemix/version.go @@ -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 {