1
0
Fork 0
mirror of https://github.com/moby/moby.git synced 2022-11-09 12:21:53 -05:00

cluster: Avoid recursive RLock

GetTasks can call GetService and GetNode with the read lock held. These
methods try to aquire the read side of the same lock. According to the
sync package documentation, this is not safe:

> If a goroutine holds a RWMutex for reading, it must not expect this or
> any other goroutine to be able to also take the read lock until the
> first read lock is released. In particular, this prohibits recursive
> read locking. This is to ensure that the lock eventually becomes
> available; a blocked Lock call excludes new readers from acquiring the
> lock.

Fix GetTasks to use the lower-level getService and getNode methods
instead. Also, use lockedManagerAction to simplify GetTasks.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
This commit is contained in:
Aaron Lehmann 2017-07-24 09:52:31 -07:00
parent 4ac4c8ef4b
commit bd4f66c8f1

View file

@ -11,57 +11,50 @@ import (
// GetTasks returns a list of tasks matching the filter options. // GetTasks returns a list of tasks matching the filter options.
func (c *Cluster) GetTasks(options apitypes.TaskListOptions) ([]types.Task, error) { func (c *Cluster) GetTasks(options apitypes.TaskListOptions) ([]types.Task, error) {
c.mu.RLock() var r *swarmapi.ListTasksResponse
defer c.mu.RUnlock()
state := c.currentNodeState() if err := c.lockedManagerAction(func(ctx context.Context, state nodeState) error {
if !state.IsActiveManager() { filterTransform := func(filter filters.Args) error {
return nil, c.errNoManager(state) if filter.Include("service") {
} serviceFilters := filter.Get("service")
for _, serviceFilter := range serviceFilters {
filterTransform := func(filter filters.Args) error { service, err := getService(ctx, state.controlClient, serviceFilter, false)
if filter.Include("service") { if err != nil {
serviceFilters := filter.Get("service") return err
for _, serviceFilter := range serviceFilters { }
service, err := c.GetService(serviceFilter, false) filter.Del("service", serviceFilter)
if err != nil { filter.Add("service", service.ID)
return err
} }
filter.Del("service", serviceFilter)
filter.Add("service", service.ID)
} }
} if filter.Include("node") {
if filter.Include("node") { nodeFilters := filter.Get("node")
nodeFilters := filter.Get("node") for _, nodeFilter := range nodeFilters {
for _, nodeFilter := range nodeFilters { node, err := getNode(ctx, state.controlClient, nodeFilter)
node, err := c.GetNode(nodeFilter) if err != nil {
if err != nil { return err
return err }
filter.Del("node", nodeFilter)
filter.Add("node", node.ID)
} }
filter.Del("node", nodeFilter)
filter.Add("node", node.ID)
} }
if !filter.Include("runtime") {
// default to only showing container tasks
filter.Add("runtime", "container")
filter.Add("runtime", "")
}
return nil
} }
if !filter.Include("runtime") {
// default to only showing container tasks filters, err := newListTasksFilters(options.Filters, filterTransform)
filter.Add("runtime", "container") if err != nil {
filter.Add("runtime", "") return err
} }
return nil
}
filters, err := newListTasksFilters(options.Filters, filterTransform) r, err = state.controlClient.ListTasks(
if err != nil { ctx,
return nil, err &swarmapi.ListTasksRequest{Filters: filters})
} return err
}); err != nil {
ctx, cancel := c.getRequestContext()
defer cancel()
r, err := state.controlClient.ListTasks(
ctx,
&swarmapi.ListTasksRequest{Filters: filters})
if err != nil {
return nil, err return nil, err
} }