Skip to content

Commit 9cca421

Browse files
AerexAerex
authored
Merge pull request #366 from IBM-Cloud/dev
Release 1.0.2
2 parents d415846 + fc7de28 commit 9cca421

File tree

12 files changed

+105
-36
lines changed

12 files changed

+105
-36
lines changed

‎.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
vendor/*
2-
!vendor/vendor.json
2+
!vendor/vendor.json
3+
.vscode

‎.secrets.baseline

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"exclude": {
3-
"files": "^.secrets.baseline$|go.sum",
3+
"files": "^.secrets.baseline$|go.sum|.nancy-ignore",
44
"lines": null
55
},
66
"generated_at": "2022-07-08T20:25:00Z",

‎.travis.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
language: go
22
dist: bionic
33
go:
4-
- '1.16.x'
54
- '1.17.x'
65
addons:
76
apt:

‎bluemix/configuration/core_config/bx_config.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ func (c *bxConfig) init() {
119119
}
120120

121121
func (c *bxConfig) read(cb func()) {
122-
c.lock.RLock()
123-
defer c.lock.RUnlock()
122+
/* concurrency note: init() calls the persistor's Load(), which has a flock,
123+
via lockedRead() and lockedWrite(), surrounding the critical sections */
124124

125125
c.init()
126126

‎bluemix/configuration/core_config/cf_config.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ type cfConfig struct {
8686
data *CFConfigData
8787
persistor configuration.Persistor
8888
initOnce *sync.Once
89-
lock sync.RWMutex
89+
lock sync.Mutex
9090
onError func(error)
9191
}
9292

@@ -119,9 +119,8 @@ func (c *cfConfig) init() {
119119
}
120120

121121
func (c *cfConfig) read(cb func()) {
122-
c.lock.RLock()
123-
defer c.lock.RUnlock()
124-
122+
/* concurrency note: init() calls the persistor's Load(), which has a flock,
123+
via lockedRead() and lockedWrite(), surrounding the critical sections */
125124
c.init()
126125

127126
cb()

‎bluemix/configuration/persistor.go

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package configuration
22

33
import (
4+
"context"
45
"io/ioutil"
56
"os"
67
"path/filepath"
8+
"time"
79

810
"github.com/IBM-Cloud/ibm-cloud-cli-sdk/common/file_helpers"
11+
"github.com/gofrs/flock"
912
)
1013

1114
const (
@@ -25,33 +28,71 @@ type Persistor interface {
2528
}
2629

2730
type DiskPersistor struct {
28-
filePath string
31+
filePath string
32+
fileLock *flock.Flock
33+
parentContext context.Context
2934
}
3035

3136
func NewDiskPersistor(path string) DiskPersistor {
3237
return DiskPersistor{
33-
filePath: path,
38+
filePath: path,
39+
fileLock: flock.New(path),
40+
parentContext: context.Background(),
3441
}
3542
}
3643

3744
func (dp DiskPersistor) Exists() bool {
3845
return file_helpers.FileExists(dp.filePath)
3946
}
4047

48+
func (dp *DiskPersistor) lockedRead(data DataInterface) error {
49+
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
50+
specified after the deferred call to cancelLockCtx). 30 appears to be a conventional value for a parent context passed to TryLockContext, as per docs */
51+
defer cancelLockCtx()
52+
_, lockErr := dp.fileLock.TryLockContext(lockCtx, 100*time.Millisecond) /* provide a file lock just while dp.read is called, because it calls an unmarshaling function
53+
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) */
54+
defer dp.fileLock.Unlock()
55+
if lockErr != nil {
56+
return lockErr
57+
}
58+
readErr := dp.read(data)
59+
if readErr != nil {
60+
return readErr
61+
}
62+
return nil
63+
}
64+
4165
func (dp DiskPersistor) Load(data DataInterface) error {
42-
err := dp.read(data)
66+
err := dp.lockedRead(data)
4367
if os.IsPermission(err) {
4468
return err
4569
}
4670

47-
if err != nil {
48-
err = dp.write(data)
71+
if err != nil { /* would happen if there was nothing to read (EOF) */
72+
err = dp.lockedWrite(data)
4973
}
5074
return err
5175
}
5276

77+
func (dp DiskPersistor) lockedWrite(data DataInterface) error {
78+
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
79+
specified after the deferred call to cancelLockCtx). 30 appears to be a conventional value for a parent context passed to TryLockContext, as per docs */
80+
defer cancelLockCtx()
81+
_, lockErr := dp.fileLock.TryLockContext(lockCtx, 100*time.Millisecond) /* provide a file lock just while dp.read is called, because it calls an unmarshaling function
82+
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) */
83+
defer dp.fileLock.Unlock()
84+
if lockErr != nil {
85+
return lockErr
86+
}
87+
writeErr := dp.write(data)
88+
if writeErr != nil {
89+
return writeErr
90+
}
91+
return nil
92+
}
93+
5394
func (dp DiskPersistor) Save(data DataInterface) error {
54-
return dp.write(data)
95+
return dp.lockedWrite(data)
5596
}
5697

5798
func (dp DiskPersistor) read(data DataInterface) error {

‎bluemix/version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package bluemix
33
import "fmt"
44

55
// Version is the SDK version
6-
var Version = VersionType{Major: 1, Minor: 0, Build: 1}
6+
var Version = VersionType{Major: 1, Minor: 0, Build: 2}
77

88
// VersionType describe version info
99
type VersionType struct {

‎common/downloader/file_downloader.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,13 @@ func (d *FileDownloader) DownloadTo(url string, outputName string) (dest string,
5858
if err != nil {
5959
return "", 0, err
6060
}
61-
defer resp.Body.Close()
61+
62+
defer func() error {
63+
if err := resp.Body.Close(); err != nil {
64+
return err
65+
}
66+
return nil
67+
}()
6268

6369
if resp.StatusCode != 200 {
6470
return "", 0, fmt.Errorf("Unexpected response code %d", resp.StatusCode)

‎common/rest/client.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,12 @@ func (c *Client) DoWithContext(ctx context.Context, r *Request, respV interface{
6767
if err != nil {
6868
return resp, err
6969
}
70-
defer resp.Body.Close()
70+
defer func() error {
71+
if err := resp.Body.Close(); err != nil {
72+
return err
73+
}
74+
return nil
75+
}()
7176

7277
if resp.StatusCode < 200 || resp.StatusCode > 299 {
7378
raw, err := ioutil.ReadAll(resp.Body)

‎docs/plugin_developer_guide.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ IBM Cloud CLI SDK provides a set of APIs to register and manage plug-ins. It als
7474

7575
**Understanding the fields in this `plugin.PluginMetadata` struct:**
7676
- _Name_: The name of plug-in. It will be displayed when using `ibmcloud plugin list` command or can be used to uninstall the plug-in through `ibmcloud plugin uninstall` command.
77+
- It is **strongly** encouraged to use a name that best describes the service the plug-in provides.
78+
- _Aliases_: A list of short names of the plug-in that can be used as a stand-in for installing, updating, uninstalling and using the plug-in.
79+
- It is strongly recommended that you have at least one alias to improve the usability of the plug-in.
7780
- _Version_: The version of plug-in.
7881
- _MinCliVersion_: The minimal version of IBM Cloud CLI required by the plug-in.
7982
- _PrivateEndpointSupported_: Indicates if the plug-in is designed to also be used over the private network.

0 commit comments

Comments
 (0)