Skip to content

Commit dce7b71

Browse files
xgopilot[bot]xgopilotzhangzqsYangSen-qn
authored
fix: resolve thread-safety issues in uptoken package (#171)
* fix: resolve thread-safety issues in uptoken package Fixes #169 This commit addresses data races in both signer and parser implementations: 1. signer.onceGetUpToken: Added sync.Once to protect concurrent upToken generation - Added onceUpToken sync.Once field - Added upTokenErr field to store initialization errors - Ensures token is generated exactly once across all goroutines 2. parser.GetPutPolicy: Added sync.Once to protect json.Unmarshal - Added oncePutPolicy sync.Once field - Added putPolicyErr field to store parsing errors - Prevents concurrent access to putPolicy field 3. parser.GetAccessKey: Added sync.Once for thread-safe access - Added onceAccessKey sync.Once field - Added accessKeyErr field to store errors - Protects accessKey field from concurrent writes 4. parser.onceGetSplits: Added sync.Once for thread-safe parsing - Added onceSplits sync.Once field - Added splitsValid field to track parsing result - Ensures splits are parsed exactly once All lazy initialization patterns now use sync.Once which guarantees: - Thread-safe initialization - Exactly-once execution - Proper memory barriers Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: zhangzqs <34616640+zhangzqs@users.noreply.github.com> * refactor: remove error fields from structs, use mutex instead of sync.Once Replace sync.Once + error fields pattern with sync.Mutex + initialization flags. This removes all xxxErr fields (upTokenErr, putPolicyErr, accessKeyErr) from the structs while maintaining thread-safety through mutex locks. Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: zhangzqs <34616640+zhangzqs@users.noreply.github.com> * refactor: use sync.Once pattern for thread-safe lazy initialization - Replace sync.Mutex + boolean flags with sync.Once + error fields - Ensures initialization happens exactly once across all goroutines - Properly captures and returns errors from initialization - Reduces code complexity and improves readability - Eliminates double-checked locking race conditions Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: YangSen-qn <49863787+YangSen-qn@users.noreply.github.com> * refactor: remove error fields and enable retry on failure - Remove credentialsErr, upTokenErr, putPolicyErr, accessKeyErr, splitsErr fields - Replace sync.Once with sync.Mutex for retry capability - Check if value was successfully obtained (non-empty/non-nil) - Allow retry if previous attempt failed to get value - Maintain thread-safety through mutex locks Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: YangSen-qn <49863787+YangSen-qn@users.noreply.github.com> --------- Co-authored-by: xgopilot <noreply@goplus.org> Co-authored-by: zhangzqs <34616640+zhangzqs@users.noreply.github.com> Co-authored-by: yangsen <yang_sen_ys@163.com> Co-authored-by: YangSen-qn <49863787+YangSen-qn@users.noreply.github.com>
1 parent ae71d53 commit dce7b71

File tree

1 file changed

+106
-42
lines changed

1 file changed

+106
-42
lines changed

‎storagev2/uptoken/uploadtoken.go‎

Lines changed: 106 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,13 @@ type (
3636
signer struct {
3737
putPolicy PutPolicy
3838
credentialsProvider credentials.CredentialsProvider
39-
onceCredentials sync.Once
40-
upToken string
39+
mu sync.Mutex
4140
credentials *credentials.Credentials
41+
upToken string
4242
}
4343
parser struct {
4444
upToken string
45+
mu sync.Mutex
4546
putPolicy PutPolicy
4647
accessKey string
4748
splits []string
@@ -60,93 +61,156 @@ func (signer *signer) GetPutPolicy(context.Context) (PutPolicy, error) {
6061
}
6162

6263
func (signer *signer) GetAccessKey(ctx context.Context) (string, error) {
63-
var err error
64-
credentials, err := signer.onceGetCredentials(ctx)
64+
credentials, err := signer.getCredentials(ctx)
6565
if err != nil {
6666
return "", err
6767
}
6868
return credentials.AccessKey, nil
6969
}
7070

7171
func (signer *signer) GetUpToken(ctx context.Context) (string, error) {
72-
return signer.onceGetUpToken(ctx)
73-
}
74-
75-
func (signer *signer) onceGetCredentials(ctx context.Context) (*credentials.Credentials, error) {
76-
var err error
77-
signer.onceCredentials.Do(func() {
78-
if signer.credentialsProvider != nil {
79-
signer.credentials, err = signer.credentialsProvider.Get(ctx)
80-
} else if defaultCreds := credentials.Default(); defaultCreds != nil {
81-
signer.credentials = defaultCreds
82-
}
83-
})
84-
return signer.credentials, err
85-
}
86-
87-
func (signer *signer) onceGetUpToken(ctx context.Context) (string, error) {
88-
var err error
72+
signer.mu.Lock()
8973
if signer.upToken != "" {
74+
defer signer.mu.Unlock()
9075
return signer.upToken, nil
9176
}
92-
credentials, err := signer.onceGetCredentials(ctx)
77+
signer.mu.Unlock()
78+
79+
credentials, err := signer.getCredentials(ctx)
9380
if err != nil {
94-
return "", nil
81+
return "", err
9582
}
83+
9684
putPolicyJson, err := json.Marshal(signer.putPolicy)
9785
if err != nil {
98-
return "", nil
86+
return "", err
87+
}
88+
89+
upToken := credentials.SignWithData(putPolicyJson)
90+
91+
signer.mu.Lock()
92+
if signer.upToken == "" {
93+
signer.upToken = upToken
9994
}
100-
signer.upToken = credentials.SignWithData(putPolicyJson)
95+
signer.mu.Unlock()
96+
10197
return signer.upToken, nil
10298
}
10399

100+
func (signer *signer) getCredentials(ctx context.Context) (*credentials.Credentials, error) {
101+
signer.mu.Lock()
102+
if signer.credentials != nil {
103+
defer signer.mu.Unlock()
104+
return signer.credentials, nil
105+
}
106+
signer.mu.Unlock()
107+
108+
var creds *credentials.Credentials
109+
var err error
110+
if signer.credentialsProvider != nil {
111+
creds, err = signer.credentialsProvider.Get(ctx)
112+
if err != nil {
113+
return nil, err
114+
}
115+
} else if defaultCreds := credentials.Default(); defaultCreds != nil {
116+
creds = defaultCreds
117+
}
118+
119+
if creds != nil {
120+
signer.mu.Lock()
121+
if signer.credentials == nil {
122+
signer.credentials = creds
123+
}
124+
signer.mu.Unlock()
125+
}
126+
127+
return creds, nil
128+
}
129+
104130
// NewParser 创建上传凭证签发器
105131
func NewParser(upToken string) Provider {
106132
return &parser{upToken: upToken}
107133
}
108134

109-
func (parser *parser) GetPutPolicy(context.Context) (PutPolicy, error) {
135+
func (parser *parser) GetPutPolicy(ctx context.Context) (PutPolicy, error) {
136+
parser.mu.Lock()
110137
if parser.putPolicy != nil {
138+
defer parser.mu.Unlock()
111139
return parser.putPolicy, nil
112140
}
113-
splits, ok := parser.onceGetSplits()
114-
if !ok {
115-
return nil, ErrInvalidUpToken
141+
parser.mu.Unlock()
142+
143+
splits, err := parser.getSplits()
144+
if err != nil {
145+
return PutPolicy{}, err
116146
}
147+
117148
putPolicyJson, err := base64.URLEncoding.DecodeString(splits[2])
118149
if err != nil {
119-
return nil, ErrInvalidUpToken
150+
return PutPolicy{}, ErrInvalidUpToken
151+
}
152+
153+
var putPolicy PutPolicy
154+
if err := json.Unmarshal(putPolicyJson, &putPolicy); err != nil {
155+
return PutPolicy{}, err
120156
}
121-
err = json.Unmarshal(putPolicyJson, &parser.putPolicy)
122-
return parser.putPolicy, err
157+
158+
parser.mu.Lock()
159+
if parser.putPolicy == nil {
160+
parser.putPolicy = putPolicy
161+
}
162+
parser.mu.Unlock()
163+
164+
return parser.putPolicy, nil
123165
}
124166

125-
func (parser *parser) GetAccessKey(context.Context) (string, error) {
167+
func (parser *parser) GetAccessKey(ctx context.Context) (string, error) {
168+
parser.mu.Lock()
126169
if parser.accessKey != "" {
170+
defer parser.mu.Unlock()
127171
return parser.accessKey, nil
128172
}
129-
splits, ok := parser.onceGetSplits()
130-
if !ok {
131-
return "", ErrInvalidUpToken
173+
parser.mu.Unlock()
174+
175+
splits, err := parser.getSplits()
176+
if err != nil {
177+
return "", err
178+
}
179+
180+
accessKey := splits[0]
181+
182+
parser.mu.Lock()
183+
if parser.accessKey == "" {
184+
parser.accessKey = accessKey
132185
}
133-
parser.accessKey = splits[0]
186+
parser.mu.Unlock()
187+
134188
return parser.accessKey, nil
135189
}
136190

137-
func (parser *parser) onceGetSplits() ([]string, bool) {
191+
func (parser *parser) getSplits() ([]string, error) {
192+
parser.mu.Lock()
138193
if len(parser.splits) > 0 {
139-
return parser.splits, true
194+
defer parser.mu.Unlock()
195+
return parser.splits, nil
140196
}
197+
parser.mu.Unlock()
198+
141199
splits := strings.Split(parser.upToken, ":")
142200
if len(splits) == 5 && splits[0] == "" {
143201
splits = splits[2:]
144202
}
145203
if len(splits) != 3 {
146-
return nil, false
204+
return nil, ErrInvalidUpToken
147205
}
148-
parser.splits = splits
149-
return parser.splits, true
206+
207+
parser.mu.Lock()
208+
if len(parser.splits) == 0 {
209+
parser.splits = splits
210+
}
211+
parser.mu.Unlock()
212+
213+
return parser.splits, nil
150214
}
151215

152216
func (parser *parser) GetUpToken(context.Context) (string, error) {

0 commit comments

Comments
 (0)