Skip to content

Conversation

@xgopilot
Copy link

@xgopilot xgopilot bot commented Nov 6, 2025

Requested by @zhangzqs

Summary

Fixed a critical thread-safety issue in the storagev2/uptoken package where concurrent calls to GetUpToken() caused data races during parallel file uploads.

Problem

The data race detector identified concurrent read/write operations on signer.upToken field:

  • Read at line 89: if signer.upToken != ""
  • Write at line 100: signer.upToken = credentials.SignWithData(putPolicyJson)

When multiple goroutines called GetUpToken() simultaneously (e.g., during UploadDirectory operations), they would:

  1. Both read the empty upToken field
  2. Both proceed to generate and write the token
  3. Create a data race condition

Solution

Implemented thread-safe token initialization using sync.Once:

  • Added onceUpToken sync.Once field to the signer struct
  • Refactored onceGetUpToken() to use sync.Once.Do() for initialization
  • This ensures token generation happens exactly once, even under concurrent access

The fix follows the same pattern already used in onceGetCredentials() method, maintaining consistency with the existing codebase.

Testing

The original issue was detected during concurrent upload operations in UploadDirectory. This fix ensures that:

  • Token generation is thread-safe
  • Multiple concurrent calls return the same cached token
  • No performance degradation (tokens are still cached after first generation)

Fixes #169

Generated with codeagent

Fixed thread-safety issue in storagev2/uptoken package where concurrent
calls to GetUpToken() caused data races. The signer.onceGetUpToken()
method was checking and writing to signer.upToken without synchronization.

Changes:
- Added onceUpToken sync.Once field to signer struct
- Refactored onceGetUpToken() to use sync.Once.Do() for thread-safe
  initialization of upToken

This ensures token generation happens exactly once even when called
concurrently by multiple goroutines during UploadDirectory operations.

Fixes #169

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: zhangzqs <34616640+zhangzqs@users.noreply.github.com>
@qiniu-prow
Copy link

qiniu-prow bot commented Nov 6, 2025

Hi @xgopilot[bot]. Thanks for your PR.

I'm waiting for a qiniu member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

storagev2包中的uptoken.NewSigner构造出的uptoken.Provider存在线程不安全的问题

2 participants