Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: remove hold WaitGroup in TestConcurrentFetch#19617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
spikecurtis merged 1 commit intomainfromspike/internal-950-data-race
Sep 3, 2025

Conversation

spikecurtis
Copy link
Contributor

@spikecurtisspikecurtis commentedAug 28, 2025
edited
Loading

Fixes:coder/internal#950

Pretty sure the intention of thehold wait group is to try to get the two goroutines that the test starts running at the same time. But, that should be the case for two goroutines started anyway.

The use ofhold doesn't actually guarantee concurrent execution ofAcquire, just that both goroutines get as far asDone() --- the go scheduler could run them serially without incident.

So I've chosen to just remove the use ofhold to simplify.

But, for posterity, the data race was due to incrementing by 1 in the loop along with the goroutine that calls Done. You could increment by 1 and then back down to 0 before the second iteration of the loop starts. This then causes a data race with callingWait() in the first goroutine andAdd() in the second iteration. c.f.https://pkg.go.dev/sync#WaitGroup.Add

@spikecurtisGraphite App
Copy link
ContributorAuthor

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

@spikecurtisspikecurtis marked this pull request as ready for reviewAugust 28, 2025 13:28
@spikecurtisspikecurtisforce-pushed thespike/internal-950-data-race branch fromeb6d892 toc40cee0CompareAugust 28, 2025 13:29
Copy link
Member

@aslilacaslilac left a comment

Choose a reason for hiding this comment

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

thanks for waiting for me to get back from vacation for the review 😄

@spikecurtisspikecurtis merged commitee35ad3 intomainSep 3, 2025
29 checks passed
@spikecurtisspikecurtis deleted the spike/internal-950-data-race branchSeptember 3, 2025 05:25
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsSep 3, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@aslilacaslilacaslilac approved these changes

Assignees

@spikecurtisspikecurtis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

flake: data race in TestConcurrentFetch
2 participants
@spikecurtis@aslilac

[8]ページ先頭

©2009-2025 Movatter.jp