From 34a93a0c2c3083f47c7262d811680d3974d02de9 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Mon, 5 Jun 2023 12:16:25 +0100 Subject: [PATCH] task: don't `close()` io before `cancel()` The contract for `cio/io.go/IO` states that a call to `Close()` will always be preceded by a call to `Cancel()` - https://github.com/containerd/containerd/blob/f3a07934b49bf142925a5913e3e19f3528eda0d2/cio/io.go#L59 which isn't being held up here. Furthermore, the call to `Close()` here makes the subsequent `Wait()` moot, and causes issues to consumers (see: https://github.com/moby/moby/issues/45689) It seems from https://github.com/containerd/containerd/blob/f3a07934b49bf142925a5913e3e19f3528eda0d2/task.go#L338 that the `Close()` should be called there, the call removed in this commit is unnecessary/erroneous. We leave the `Close()` call on Windows only since this was introduced in https://github.com/containerd/containerd/pull/5974 to address https://github.com/containerd/containerd/issues/5621. Signed-off-by: Laura Brehm --- task.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/task.go b/task.go index 7c5de6c76..97e1b5530 100644 --- a/task.go +++ b/task.go @@ -325,7 +325,16 @@ func (t *task) Delete(ctx context.Context, opts ...ProcessDeleteOpts) (*ExitStat return nil, fmt.Errorf("task must be stopped before deletion: %s: %w", status.Status, errdefs.ErrFailedPrecondition) } if t.io != nil { - t.io.Close() + // io.Wait locks for restored tasks on Windows unless we call + // io.Close first (https://github.com/containerd/containerd/issues/5621) + // in other cases, preserve the contract and let IO finish before closing + if t.client.runtime == fmt.Sprintf("%s.%s", plugin.RuntimePlugin, "windows") { + t.io.Close() + } + // io.Cancel is used to cancel the io goroutine while it is in + // fifo-opening state. It does not stop the pipes since these + // should be closed on the shim's side, otherwise we might lose + // data from the container! t.io.Cancel() t.io.Wait() }