Skip to content

Commit

Permalink
libct: fix locking in Start/Run/Exec
Browse files Browse the repository at this point in the history
1. The code to call c.exec from c.Run was initially added by commit
   3aacff6. At the time, there was a lock in c.Run. That lock was
   removed by commit bd3c4f8, which resulted in part of c.Run executing
   without the lock.

2. All the Start/Run/Exec calls were a mere wrappers for start/run/exec
   adding a lock, but some more code crept into Start at some point,
   e.g. by commits 805b8c7 and 108ee85. Since the reason mentioned in
   commit 805b8c7 is no longer true after refactoring, we can fix this.

Fix both issues by moving code out of wrappers, and adding locking into
c.Run.

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Jun 11, 2024
1 parent 304a4c0 commit e3e1072
Showing 1 changed file with 18 additions and 16 deletions.
34 changes: 18 additions & 16 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,28 +204,16 @@ func (c *Container) Set(config configs.Config) error {
func (c *Container) Start(process *Process) error {
c.m.Lock()
defer c.m.Unlock()
if c.config.Cgroups.Resources.SkipDevices {
return errors.New("can't start container with SkipDevices set")
}
if process.Init {
if err := c.createExecFifo(); err != nil {
return err
}
}
if err := c.start(process); err != nil {
if process.Init {
c.deleteExecFifo()
}
return err
}
return nil
return c.start(process)
}

// Run immediately starts the process inside the container. Returns an error if
// the process fails to start. It does not block waiting for the exec fifo
// after start returns but opens the fifo after start returns.
func (c *Container) Run(process *Process) error {
if err := c.Start(process); err != nil {
c.m.Lock()
defer c.m.Unlock()
if err := c.start(process); err != nil {
return err
}
if process.Init {
Expand Down Expand Up @@ -314,6 +302,20 @@ type openResult struct {
}

func (c *Container) start(process *Process) (retErr error) {
if c.config.Cgroups.Resources.SkipDevices {
return errors.New("can't start container with SkipDevices set")
}
if process.Init {
if err := c.createExecFifo(); err != nil {
return err
}
defer func() {
if retErr != nil {
c.deleteExecFifo()
}
}()
}

parent, err := c.newParentProcess(process)
if err != nil {
return fmt.Errorf("unable to create new parent process: %w", err)
Expand Down

0 comments on commit e3e1072

Please sign in to comment.