From b75246202ab9b1e5bb94c377f90db8ed38cfa0e0 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Thu, 21 Apr 2022 15:33:23 -0400 Subject: [PATCH] Stop locking container exec store while starting The daemon.containerd.Exec call does not access or mutate the container's ExecCommands store in any way, and locking the exec config is sufficient to synchronize with the event-processing loop. Locking the ExecCommands store while starting the exec process only serves to block unrelated operations on the container for an extended period of time. Convert the Store struct's mutex to an unexported field to prevent this from regressing in the future. Signed-off-by: Cory Snider --- daemon/exec.go | 3 --- daemon/exec/exec.go | 22 +++++++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/daemon/exec.go b/daemon/exec.go index 4675ee4557..5f609d4676 100644 --- a/daemon/exec.go +++ b/daemon/exec.go @@ -276,18 +276,15 @@ func (daemon *Daemon) ContainerExecStart(ctx context.Context, name string, optio // Synchronize with libcontainerd event loop ec.Lock() - c.ExecCommands.Lock() systemPid, err := daemon.containerd.Exec(ctx, c.ID, ec.ID, p, cStdin != nil, ec.InitializeStdio) // the exec context should be ready, or error happened. // close the chan to notify readiness close(ec.Started) if err != nil { - c.ExecCommands.Unlock() ec.Unlock() return translateContainerdStartErr(ec.Entrypoint, ec.SetExitCode, err) } ec.Pid = systemPid - c.ExecCommands.Unlock() ec.Unlock() select { diff --git a/daemon/exec/exec.go b/daemon/exec/exec.go index 2cf1833d7d..099ae3672c 100644 --- a/daemon/exec/exec.go +++ b/daemon/exec/exec.go @@ -93,7 +93,7 @@ func (c *Config) SetExitCode(code int) { // Store keeps track of the exec configurations. type Store struct { byID map[string]*Config - sync.RWMutex + mu sync.RWMutex } // NewStore initializes a new exec store. @@ -105,44 +105,44 @@ func NewStore() *Store { // Commands returns the exec configurations in the store. func (e *Store) Commands() map[string]*Config { - e.RLock() + e.mu.RLock() byID := make(map[string]*Config, len(e.byID)) for id, config := range e.byID { byID[id] = config } - e.RUnlock() + e.mu.RUnlock() return byID } // Add adds a new exec configuration to the store. func (e *Store) Add(id string, Config *Config) { - e.Lock() + e.mu.Lock() e.byID[id] = Config - e.Unlock() + e.mu.Unlock() } // Get returns an exec configuration by its id. func (e *Store) Get(id string) *Config { - e.RLock() + e.mu.RLock() res := e.byID[id] - e.RUnlock() + e.mu.RUnlock() return res } // Delete removes an exec configuration from the store. func (e *Store) Delete(id string, pid int) { - e.Lock() + e.mu.Lock() delete(e.byID, id) - e.Unlock() + e.mu.Unlock() } // List returns the list of exec ids in the store. func (e *Store) List() []string { var IDs []string - e.RLock() + e.mu.RLock() for id := range e.byID { IDs = append(IDs, id) } - e.RUnlock() + e.mu.RUnlock() return IDs }