Lock agent access in addDriverWatches#1457
Conversation
Signed-off-by: Alessandro Boch <aboch@docker.com>
| for _, tableName := range n.driverTables { | ||
| ch, cancel := c.agent.networkDB.Watch(tableName, n.ID(), "") | ||
| c.Lock() | ||
| if c.agent == nil { |
There was a problem hiding this comment.
You need to retrieve agent within lock and only use that after. Not c.agent anymore.
There was a problem hiding this comment.
But I want to keep the current logic where the lock is held until after c.agent.driverCancelFuncs map is modified.
If you are suggesting to add a local variable even thoug we will use it inside a locked scope, I do not see the need.
There was a problem hiding this comment.
No the problem is the c.agent.networkDB and c.agent.driverCancelFuncs access below will crash if c.agent is made nil during that time. That is the actual root cause for this crash.
There was a problem hiding this comment.
c.agent cannot be made nil if we hold the lock.
This change adds some coordination, so that when we access c.agent and we assert it is not nil, it we can modify its members assured it cannot be made nil in the meanwhile.
There was a problem hiding this comment.
Yeah missed that you are still holding the lock when you are access the agent. Changes look good.
|
LGTM |
4 participants
Related to moby/moby#25848
Signed-off-by: Alessandro Boch aboch@docker.com