From ffce32f4fe1be53f37ff3480c8b7e73de9e9436d Mon Sep 17 00:00:00 2001 From: Rob Watson Date: Wed, 26 Mar 2025 13:35:07 +0100 Subject: [PATCH] fix(container): handle race condition on ContainerWait When a destination is removed, its container is stopped and removed. This preempts the ContainerWait call in the destination loop, which then tries to call ContainerInspect to infer if the container is restarting or not. It was already known that this call is prone to race conditions. This commit handles an additional case where the container has already been completely removed by the time the call is made; we can safely infer that it is not restarting. --- internal/container/container.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/container/container.go b/internal/container/container.go index 6106820..72893b9 100644 --- a/internal/container/container.go +++ b/internal/container/container.go @@ -21,6 +21,7 @@ import ( "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/image" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/errdefs" ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -338,18 +339,22 @@ func (a *Client) runContainerLoop( respC, errC := a.apiClient.ContainerWait(ctx, containerID, container.WaitConditionNextExit) select { case resp := <-respC: + var restarting bool // Check if the container is restarting. If it is not then we don't // want to wait for it again and can return early. ctr, err := a.apiClient.ContainerInspect(ctx, containerID) - if err != nil { + // Race conidition: the container may already have been removed. + if errdefs.IsNotFound(err) { + // ignore error but do not restart + } else if err != nil { a.logger.Error("Error inspecting container", "err", err, "id", shortID(containerID)) containerErrC <- err return + // Race condition: the container may have already restarted. + } else if ctr.State.Status == domain.ContainerStatusRestarting || ctr.State.Status == domain.ContainerStatusRunning { + restarting = true } - // Race condition: the container may have already restarted. - restarting := ctr.State.Status == domain.ContainerStatusRestarting || ctr.State.Status == domain.ContainerStatusRunning - containerRespC <- containerWaitResponse{WaitResponse: resp, restarting: restarting} if !restarting { return