diff --git a/libpod/container_api.go b/libpod/container_api.go index 8a309c4cf9..02e35199ba 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -40,7 +40,10 @@ func (c *Container) Init(ctx context.Context, recursive bool) error { return err } } + return c.initUnlocked(ctx, recursive) +} +func (c *Container) initUnlocked(ctx context.Context, recursive bool) error { if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateStopped, define.ContainerStateExited) { return fmt.Errorf("container %s has already been created in runtime: %w", c.ID(), define.ErrCtrStateInvalid) } @@ -342,25 +345,36 @@ func (c *Container) HTTPAttach(r *http.Request, w http.ResponseWriter, streams * close(hijackDone) }() + locked := false if !c.batched { + locked = true c.lock.Lock() + defer func() { + if locked { + c.lock.Unlock() + } + }() if err := c.syncContainer(); err != nil { - c.lock.Unlock() - return err } - // We are NOT holding the lock for the duration of the function. - c.lock.Unlock() } - - if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) { - return fmt.Errorf("can only attach to created or running containers: %w", define.ErrCtrStateInvalid) + // For Docker compatibility, we need to re-initialize containers in these states. + if c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited, define.ContainerStateStopped) { + if err := c.initUnlocked(r.Context(), c.config.Pod != ""); err != nil { + return fmt.Errorf("preparing container %s for attach: %w", c.ID(), err) + } + } else if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) { + return fmt.Errorf("can only attach to created or running containers - currently in state %s: %w", c.state.State.String(), define.ErrCtrStateInvalid) } if !streamAttach && !streamLogs { return fmt.Errorf("must specify at least one of stream or logs: %w", define.ErrInvalidArg) } + // We are NOT holding the lock for the duration of the function. + locked = false + c.lock.Unlock() + logrus.Infof("Performing HTTP Hijack attach to container %s", c.ID()) c.newContainerEvent(events.Attach) diff --git a/pkg/api/handlers/compat/containers_attach.go b/pkg/api/handlers/compat/containers_attach.go index 301be134b1..6dacdb29ee 100644 --- a/pkg/api/handlers/compat/containers_attach.go +++ b/pkg/api/handlers/compat/containers_attach.go @@ -4,11 +4,9 @@ package compat import ( "errors" - "fmt" "net/http" "github.com/containers/podman/v5/libpod" - "github.com/containers/podman/v5/libpod/define" "github.com/containers/podman/v5/pkg/api/handlers/utils" "github.com/containers/podman/v5/pkg/api/server/idle" api "github.com/containers/podman/v5/pkg/api/types" @@ -79,22 +77,6 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) { return } - state, err := ctr.State() - if err != nil { - utils.InternalServerError(w, err) - return - } - // For Docker compatibility, we need to re-initialize containers in these states. - if state == define.ContainerStateConfigured || state == define.ContainerStateExited || state == define.ContainerStateStopped { - if err := ctr.Init(r.Context(), ctr.PodID() != ""); err != nil { - utils.Error(w, http.StatusConflict, fmt.Errorf("preparing container %s for attach: %w", ctr.ID(), err)) - return - } - } else if !(state == define.ContainerStateCreated || state == define.ContainerStateRunning) { - utils.InternalServerError(w, fmt.Errorf("can only attach to created or running containers - currently in state %s: %w", state.String(), define.ErrCtrStateInvalid)) - return - } - logErr := func(e error) { logrus.Errorf("Error attaching to container %s: %v", ctr.ID(), e) }