-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix data race on migrate repository #5224
Fix data race on migrate repository #5224
Conversation
partially fix #5189 |
Codecov Report
@@ Coverage Diff @@
## master #5224 +/- ##
==========================================
- Coverage 37.54% 37.47% -0.08%
==========================================
Files 310 310
Lines 45929 45934 +5
==========================================
- Hits 17245 17213 -32
- Misses 26214 26244 +30
- Partials 2470 2477 +7
Continue to review full report at Codecov.
|
@lunny LGTM. To prevent such bugs in the future, are the following rules correct (required)?
|
@typeless I agree with 2, but don't know why we need 1. Copy always cannot result in data race. |
@lunny regarding data race, 1. is not required. I made that assumption to be easier to reason about it. I thought the If we have two copies of |
The data race occupied when both two go routines run
repo.getOwner
and it will setrepo.Owner
. SinceUpdateRepoIndexer(repo)
will send therepo
pointer to another goroutine, this PR lets it runs after all other operations finished to resolve the data race. A complete solution to resolve this like problem is to changeUpdateRepoIndexer(neededRepoInfo)
to only copy needed information from repo.