Skip to content

Conversation

@pracucci
Copy link
Contributor

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

We're reviewing #3937 and we found a race condition introduced in pool.BucketedBytes.Put(). If the input buffer is larger than the largest bucket size, then p.usedTotal is modified outside the lock.

In this PR I'm rolling back the change done in #3937 which I think was unnecessary. Before the lock only p.sizes and p.buckets[i] are accessed: p.sizes doesn't change and p.buckets[i] (which is a sync.Pool) is thread safe.

Verification

N/A

Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

Copy link
Contributor

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💯 Thanks for fixing it.

@kakkoyun
Copy link
Contributor

The Github CI seems broken and failures are not related, so merging this one.

@kakkoyun kakkoyun merged commit 6d50805 into main Apr 22, 2021
@kakkoyun kakkoyun deleted the fix-race-condition-in-bucketed-bytes branch April 22, 2021 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants