Release memoryStore locks before filter/apply

[tonistiigi]

Rework memoryStore so that filters and apply run
on a cloned list of containers after the lock has
been released. This avoids possible deadlocks when
these filter/apply callbacks take locks for a
container.

Fixes #22732

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

[cpuguy83]

Don't we still only have pointers to the containers? Should this be a copy of each container, or is the issue with the collection itself?

[tonistiigi]

@cpuguy83 The problem is that the memoryStore lock is being held when the callbacks run and need container locks. On attach, the situation is opposite where container lock is already held and memoryStore lock is needed.

[estesp]

Nice @tonistiigi ; this will help with some of the issues I have been looking at lately around lock contention. Specific to @cpuguy83's Q, the lock on the memoryStore is to protect iteration over it from modification via add/remove. Making a copy of the list resolves that problem. Container locks still exist to protect container metadata and already have appropriate protections via the container lock, so no extra work needs to be done here.

[aaronlehmann]

This could be a good use case for https://github.com/hashicorp/go-immutable-radix. Each change to the tree results in a new tree root, but all nodes except the parents of the changed node are shared by the new tree and the old tree. By using atomic pointer swaps to update the tree root, it's a good way to do cheap copy-on-write. This is how https://github.com/hashicorp/go-memdb works.

[bukzor]

This was your only failure:

02:45:51 ----------------------------------------------------------------------
02:45:51 FAIL: docker_cli_help_test.go:15: TestHelpTextVerify.pN52_github_com_docker_docker_integration_cli.DockerSuite
02:45:51 
02:45:51 docker_cli_help_test.go:137:
02:45:51     c.Fatal(err)
02:45:51 ... Error: Should not have a blank line on ["logs"]
02:45:51 

[cpuguy83]

LGTM

[estesp]

LGTM