Skip to content

ops: fix deadlock on releasing shared mounts #1355

Merged
tonistiigi merged 3 commits intomoby:masterfrom
tonistiigi:mounts-deadlock
Feb 21, 2020
Merged

ops: fix deadlock on releasing shared mounts #1355
tonistiigi merged 3 commits intomoby:masterfrom
tonistiigi:mounts-deadlock

Conversation

@tonistiigi
Copy link
Copy Markdown
Member

fix #1322
regression from 7b1bae7#diff-0d2115e5c32fba316a7eaa987331881cR228

#1092 added another global lock to protect writes to the global map of shared cache mounts. Because the lock is taken too early if conflicts with the locking order of the release method. In case the first shared cache mount is released at the same time another mount is taken with the same key locks are held in the opposite order leading to a deadlock. This PR fixes the ordering issue and makes sure the global lock is not taken before it is needed, improving the efficiency of the mounts with locking sharing mode.

Refactored code to allow unit tests. Tests added for the deadlock case and for all of the sharing methods.

@cpuguy83

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

var cacheRefReleaseHijack func()
var cacheRefCloneHijack func()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If we move these to a context value, the tests can be run in parallel.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Extra code needed for that isn't probably worth it. clone() doesn't even take a context atm.

@tonistiigi tonistiigi added this to the v0.6.4 milestone Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deadlock on cache mounts

3 participants