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 <csnider@mirantis.com>
This commit is contained in:
Cory Snider 2022-04-21 15:33:23 -04:00
parent ce550fa9c2
commit b75246202a
2 changed files with 11 additions and 14 deletions

View file

@ -276,18 +276,15 @@ func (daemon *Daemon) ContainerExecStart(ctx context.Context, name string, optio
// Synchronize with libcontainerd event loop // Synchronize with libcontainerd event loop
ec.Lock() ec.Lock()
c.ExecCommands.Lock()
systemPid, err := daemon.containerd.Exec(ctx, c.ID, ec.ID, p, cStdin != nil, ec.InitializeStdio) systemPid, err := daemon.containerd.Exec(ctx, c.ID, ec.ID, p, cStdin != nil, ec.InitializeStdio)
// the exec context should be ready, or error happened. // the exec context should be ready, or error happened.
// close the chan to notify readiness // close the chan to notify readiness
close(ec.Started) close(ec.Started)
if err != nil { if err != nil {
c.ExecCommands.Unlock()
ec.Unlock() ec.Unlock()
return translateContainerdStartErr(ec.Entrypoint, ec.SetExitCode, err) return translateContainerdStartErr(ec.Entrypoint, ec.SetExitCode, err)
} }
ec.Pid = systemPid ec.Pid = systemPid
c.ExecCommands.Unlock()
ec.Unlock() ec.Unlock()
select { select {

View file

@ -93,7 +93,7 @@ func (c *Config) SetExitCode(code int) {
// Store keeps track of the exec configurations. // Store keeps track of the exec configurations.
type Store struct { type Store struct {
byID map[string]*Config byID map[string]*Config
sync.RWMutex mu sync.RWMutex
} }
// NewStore initializes a new exec store. // NewStore initializes a new exec store.
@ -105,44 +105,44 @@ func NewStore() *Store {
// Commands returns the exec configurations in the store. // Commands returns the exec configurations in the store.
func (e *Store) Commands() map[string]*Config { func (e *Store) Commands() map[string]*Config {
e.RLock() e.mu.RLock()
byID := make(map[string]*Config, len(e.byID)) byID := make(map[string]*Config, len(e.byID))
for id, config := range e.byID { for id, config := range e.byID {
byID[id] = config byID[id] = config
} }
e.RUnlock() e.mu.RUnlock()
return byID return byID
} }
// Add adds a new exec configuration to the store. // Add adds a new exec configuration to the store.
func (e *Store) Add(id string, Config *Config) { func (e *Store) Add(id string, Config *Config) {
e.Lock() e.mu.Lock()
e.byID[id] = Config e.byID[id] = Config
e.Unlock() e.mu.Unlock()
} }
// Get returns an exec configuration by its id. // Get returns an exec configuration by its id.
func (e *Store) Get(id string) *Config { func (e *Store) Get(id string) *Config {
e.RLock() e.mu.RLock()
res := e.byID[id] res := e.byID[id]
e.RUnlock() e.mu.RUnlock()
return res return res
} }
// Delete removes an exec configuration from the store. // Delete removes an exec configuration from the store.
func (e *Store) Delete(id string, pid int) { func (e *Store) Delete(id string, pid int) {
e.Lock() e.mu.Lock()
delete(e.byID, id) delete(e.byID, id)
e.Unlock() e.mu.Unlock()
} }
// List returns the list of exec ids in the store. // List returns the list of exec ids in the store.
func (e *Store) List() []string { func (e *Store) List() []string {
var IDs []string var IDs []string
e.RLock() e.mu.RLock()
for id := range e.byID { for id := range e.byID {
IDs = append(IDs, id) IDs = append(IDs, id)
} }
e.RUnlock() e.mu.RUnlock()
return IDs return IDs
} }