[v1.13] Add workaround for possible SetClusterProvider deadlock #30038

Open
wants to merge 1 commit into
from

Projects

None yet

6 participants

@tonistiigi
Contributor

fixes #29952

@aaronlehmann @aboch

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

@tonistiigi tonistiigi changed the title from Add workaround for possible SetClusterProvider deadlock to [v1.13] Add workaround for possible SetClusterProvider deadlock Jan 10, 2017
@tonistiigi tonistiigi Add workaround for possible SetClusterProvider deadlock
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
4b2b0eb
@tonistiigi tonistiigi added this to the 1.13.0 milestone Jan 10, 2017
@tonistiigi tonistiigi requested review from aboch and aaronlehmann Jan 10, 2017
@@ -449,7 +449,9 @@ func (daemon *Daemon) registerLink(parent, child *container.Container, alias str
// SetClusterProvider sets a component for querying the current cluster state.
func (daemon *Daemon) SetClusterProvider(clusterProvider cluster.Provider) {
daemon.clusterProvider = clusterProvider
- daemon.netController.SetClusterProvider(clusterProvider)
+ // call this in a goroutine to allow netcontroller handle this event async
@thaJeztah
thaJeztah Jan 10, 2017 Member

Can we add a TODO here if this is a workaround, and needs another approach?

@tonistiigi
tonistiigi Jan 10, 2017 Contributor

This issue has been fixed in master with #28907

@thaJeztah
thaJeztah Jan 10, 2017 Member

Ah, ignore then 👍

@tonistiigi tonistiigi added priority/P1 and removed priority/P0 labels Jan 10, 2017
@aboch
Contributor
aboch commented Jan 10, 2017

Looks good to me

@aaronlehmann

LGTM

@aboch
aboch approved these changes Jan 11, 2017 View changes
@AkihiroSuda
Contributor

question: this fix reduces the probability of deadlock, but the deadlock can still happen, right?
If so, I think it is worth commenting.

Change itself is LGTM

@tonistiigi
Contributor
tonistiigi commented Jan 11, 2017 edited

@AkihiroSuda The deadlock should not happen through SetClusterProvider. If you look at #29952 then there is one another trace that doesn't use SetClusterProvider but we think it is impossible to hit it with real-world usage. Still, this is a workaround, the issue is fixed with #28907

@thaJeztah thaJeztah modified the milestone: 1.13.1, 1.13.0 Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment