Skip to content
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

[MOD] fix fatal error: concurrent map read and map write #1506

Closed
wants to merge 1 commit into from

Conversation

cmingxu
Copy link

@cmingxu cmingxu commented Oct 14, 2016

No description provided.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" git@github.com:cmingxu/libnetwork.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Signed-off-by: kevin xu <cming.xu@gmail.com>
@kalahari
Copy link

I believe I am experiencing the issue solved in this pull, would it be of any use for me to open an issue?

...
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: time="2016-10-20T02:35:07-05:00" level=info msg="Firewalld running: false"
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: time="2016-10-20T02:35:07.261788623-05:00" level=info msg="Failed to delete real server 172.30.100.99 for vip 172.30.100.13 fwmark 330: no such process"
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: fatal error: concurrent map read and map write
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: goroutine 4061103 [running]:
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: runtime.throw(0x1f57880, 0x21)
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: /usr/local/go/src/runtime/panic.go:547 +0x90 fp=0xc82a590ca0 sp=0xc82a590c88
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: runtime.mapaccess2_faststr(0x16f7600, 0xc830803710, 0xc82cf00b40, 0x40, 0xc82030a000, 0xc82933dcc0)
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: /usr/local/go/src/runtime/hashmap_fast.go:307 +0x5b fp=0xc82a590d00 sp=0xc82a590ca0
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: github.com/docker/libnetwork.(*sandbox).isEndpointPopulated(0xc82dd1a780, 0xc827540dc0, 0x1)
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: /root/rpmbuild/BUILD/docker-engine/vendor/src/github.com/docker/libnetwork/sandbox.go:972 +0x85 fp=0xc82a590d50 sp=0xc82a590d00
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: github.com/docker/libnetwork.(*network).rmLBBackend.func1(0x7fdb9c0bf128, 0xc827540dc0, 0x0)
Oct 20 02:35:07 d9-t29-lsmo01.clarabridge.net dockerd[11172]: /root/rpmbuild/BUILD/docker-engine/vendor/src/github.com/docker/libnetwork/service_linux.go:350 +0xd1 fp=0xc82a590e38 sp=0xc82a590d50
...

@mrjana
Copy link
Contributor

mrjana commented Oct 20, 2016

@kalahari There is already an issue moby/moby#27486 and it is already fixed. It will be available in the next patch release.

Copy link
Contributor

@aboch aboch left a comment

Choose a reason for hiding this comment

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

This change does not fix the original issue.
The fix for it was pushed in #1512 and merged.

sb.Lock()
sb.RLock()
defer sb.RUnlock()

_, ok := sb.populatedEndpoints[ep.ID()]
Copy link
Contributor

Choose a reason for hiding this comment

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

The access to this map is already protected by sandbox lock.
This change does not help.

The root cause of the issue was a delete on the map run outside of sandbox lock and was fixed in #1512

@@ -80,6 +80,7 @@ type sandbox struct {
ingress bool
ndotsSet bool
sync.Mutex
sync.RWMutex
Copy link
Contributor

Choose a reason for hiding this comment

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

You adding sync.RWMutex along with the existing sync.Mutex.
From which package will all the existing calls sb.Lock()/Unlock() execute, sync.mutex or sync.RWMutex ?
If we were to accept this change, you should remove sync.Mutex.

@aboch
Copy link
Contributor

aboch commented Oct 26, 2016

Based on the comments, I will go ahead and close this PR

@aboch aboch closed this Oct 26, 2016
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.

5 participants