runtime_vm: Avoid possible deadlock on UpdateContainerStatus()

fidencio · fidencio · commit 0957c451ba62 · 2020-08-10T10:18:50.000+02:00
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 <fidencio@redhat.com> (cherry picked from commit faad1a4) (cherry picked from commit 2e92c0b)
diff --git a/internal/oci/runtime_vm.go b/internal/oci/runtime_vm.go
@@ -258,7 +258,9 @@ func (r *runtimeVM) StartContainer(c *Container) error {
 	go func() {
 		_, err = r.wait(r.ctx, c.ID(), "")
 		if err == nil {
-			err = r.UpdateContainerStatus(c)
+			if err1 := r.updateContainerStatus(c); err1 != nil {
+				logrus.Warningf("error updating container status %v", err1)
+			}
 		}
 	}()
 
@@ -576,6 +578,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 {
