cc @cpuguy83

IIRC previously similar discussion was done in #26668

CI hanging

00:14:46.880 --- FAIL: Test (335.68s)
00:14:46.882 panic: DockerSuite.TestAPINetworkConnectDisconnect test timed out after 5m0s [recovered]
00:14:46.883 	panic: DockerSuite.TestAPINetworkConnectDisconnect test timed out after 5m0s

Looks valid. That stuff is really bad.

BTW Gordon not recognizing rebuild/experimental label... 😕

Just wondering - why was it racey w/o the use of the lock? Is it because the s.* properties could change values 1/2 thru the processing of that func?

@duglin Yes, and could potentially even cause a panic.

@LK4D4 #28483 is the start of removing this lock altogether.
See master...cpuguy83:container_memdb -- this is a bit out of date but still the same basic idea of what I want to do to remove this lock.

could we not remove the lock and instead just copy the state into a local variable and then examine that? That should make it thread safe, no?

@duglin It's racey to look at the state without holding the lock, and just reading the state string is super fast. It's not about lock contention here, it's an issue of deadlocks.... in this case due to a goroutine trying to double-lock the container.

This lock is doing way way too much and it's causing lots of problems.

@cpuguy83 I've added lock in specific place with race. I understand that it's bad, but lock is still there :/

@LK4D4 is there a way to add a test for that race?

@mlaventure there is no state tests at all :/ I can write one specifically for that race, but seems like we didn't run with -race anyway.

I think the main place where this is a problem is in /info, right?
Can we just keep a running tally of states when the state change happens?
Then /info doesn't need to look at the container list at all.

@cpuguy83
The last revision of #26668 seems to have implemented that design.

However, the PR is closed because we found that we should cover docker ps as well.

@cpuguy83 can we merge this in the meantime?

I really don't like locking the container mutex here.
Opened #29554 which implements a counter for states. This is at least a stop-gap until the container object is reigned in.

what's the conclusion?
Related: #30225

I'm going to close this.