-
Notifications
You must be signed in to change notification settings - Fork 121
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 concurrent map iteration & write issue #329
Fix concurrent map iteration & write issue #329
Conversation
Solve issue: #324 |
/assign @CarlJi |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Blaxon The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @CarlJi , I just made a new commit
|
b5f944c
to
267535a
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #329 +/- ##
==========================================
- Coverage 70.76% 70.51% -0.25%
==========================================
Files 34 34
Lines 2035 2042 +7
==========================================
Hits 1440 1440
- Misses 482 489 +7
Partials 113 113
Flags with carried forward coverage won't be shown. Click here to find out more.
|
The issue was introduced in
func (l *fileStore) Remove(addr string) error
https://github.com/qiniu/goc/blob/master/pkg/cover/store.go#L107
when we set the memory server list back to file.
the list
l.memoryStore.GetAll() -> services
passed intofunc (l *fileStore) Set(services map[string][]string) error
in L175 we sign the
services
directly tol.memoryStore.servicesMap
, without copying it.while in L186 we are still using this map to do the iteration without lock. (by right should use
memoryStore.mu
.The issue will happen when there is another goroutine tries to remove the server from memoryStore. Map will clash.