Skip to content

Commit

Permalink
runtime_vm: Avoid possible deadlock on UpdateContainerStatus()
Browse files Browse the repository at this point in the history
StartContainer() function will hold the opLock, while a goroutine will
be waiting for the container to terminate. Once it happens, the
goroutine would update the container status, calling then
UpdateContainerStatus() which will try to get the opLock and,
consequently, a deadlock will happen.

In order to avoid such situation, let's create the
updateContainerStatus() helper, which actually does status update but
does not try to get the opLock, leaving it up to the caller to do so.

This new helper will be used from both StartContainer() and
UpdateContainerStatus() functions.

Signed-off-by: Fabiano Fidêncio <[email protected]>
  • Loading branch information
fidencio committed Jul 24, 2020
1 parent 34af4c7 commit aee8c15
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion internal/oci/runtime_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func (r *runtimeVM) StartContainer(c *Container) error {
go func() {
_, err := r.wait(r.ctx, c.ID(), "")
if err == nil {
if err1 := r.UpdateContainerStatus(c); err1 != nil {
if err1 := r.updateContainerStatus(c); err1 != nil {
logrus.Warningf("error updating container status %v", err1)
}
} else {
Expand Down Expand Up @@ -579,6 +579,16 @@ func (r *runtimeVM) UpdateContainerStatus(c *Container) error {
c.opLock.Lock()
defer c.opLock.Unlock()

return r.updateContainerStatus(c)
}

// updateContainerStatus is a UpdateContainerStatus helper, which actually does the container's
// status refresh.
// It does **not** Lock the container, thus it's the caller responsibility to do so, when needed.
func (r *runtimeVM) updateContainerStatus(c *Container) error {
logrus.Debug("runtimeVM.updateContainerStatus() start")
defer logrus.Debug("runtimeVM.updateContainerStatus() end")

// This can happen on restore, for example if we switch the runtime type
// for a container from "oci" to "vm" for the same runtime.
if r.task == nil {
Expand Down

0 comments on commit aee8c15

Please sign in to comment.