diff --git a/bluemix/configuration/core_config/bx_config.go b/bluemix/configuration/core_config/bx_config.go index 40fc52e..79f3b9b 100644 --- a/bluemix/configuration/core_config/bx_config.go +++ b/bluemix/configuration/core_config/bx_config.go @@ -154,6 +154,10 @@ func (c *bxConfig) writeRaw(cb func()) { err := c.persistor.Save(c.data.raw) if err != nil { + /* NOTE: the error could be triggered by a file-locking issue, + a file-unlocking issue, OR a file-writing issue; currently, the error chain + ends in `panic("configuration error: " + err.Error()"`, which is somewhat + generic but sufficient for all three of these error types */ c.onError(err) } } diff --git a/bluemix/configuration/persistor.go b/bluemix/configuration/persistor.go index 50c0bbc..da2f79c 100644 --- a/bluemix/configuration/persistor.go +++ b/bluemix/configuration/persistor.go @@ -1,11 +1,14 @@ package configuration import ( + "context" "io/ioutil" "os" "path/filepath" + "time" "github.com/IBM-Cloud/ibm-cloud-cli-sdk/common/file_helpers" + "github.com/gofrs/flock" ) const ( @@ -25,12 +28,16 @@ type Persistor interface { } type DiskPersistor struct { - filePath string + filePath string + fileLock *flock.Flock + parentContext context.Context } func NewDiskPersistor(path string) DiskPersistor { return DiskPersistor{ - filePath: path, + filePath: path, + fileLock: flock.New(path), + parentContext: context.Background(), } } @@ -44,14 +51,33 @@ func (dp DiskPersistor) Load(data DataInterface) error { return err } - if err != nil { - err = dp.write(data) + if err != nil { // strange: requiring a reading error (to allow write attempt to continue), as long as it is not a permission error + err = dp.lockedWrite(data) } return err } +func (dp DiskPersistor) lockedWrite(data DataInterface) error { + /* allotting a 3-second timeout means there can be a maximum of 5 retrials (each up to 500 ms, as + specified after the deferred call to cancelLockCtx) */ + lockCtx, cancelLockCtx := context.WithTimeout(dp.parentContext, 3*time.Second) + defer cancelLockCtx() + /* provide a file lock, in addition to the RW mutex (in calling functions), just while dp.write is called + 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) */ + _, lockErr := dp.fileLock.TryLockContext(lockCtx, 500*time.Millisecond) + if lockErr != nil { + return lockErr + } + writeErr := dp.write(data) + if writeErr != nil { + return writeErr + } + return dp.fileLock.Unlock() +} + func (dp DiskPersistor) Save(data DataInterface) error { - return dp.write(data) + return dp.lockedWrite(data) } func (dp DiskPersistor) read(data DataInterface) error { diff --git a/go.mod b/go.mod index b8bf97a..06e9f17 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( require ( github.com/BurntSushi/toml v1.1.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect + github.com/gofrs/flock v0.8.1 // indirect github.com/golang/protobuf v1.5.2 // indirect github.com/hpcloud/tail v1.0.0 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect diff --git a/go.sum b/go.sum index c732b52..ff7d80b 100644 --- a/go.sum +++ b/go.sum @@ -32,6 +32,8 @@ github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2 github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= +github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw= +github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=