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()` -f3a07934b4/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 fromf3a07934b4/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 <laurabrehm@hey.com>
This commit is contained in:
parent
f3a07934b4
commit
34a93a0c2c
11
task.go
11
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)
|
return nil, fmt.Errorf("task must be stopped before deletion: %s: %w", status.Status, errdefs.ErrFailedPrecondition)
|
||||||
}
|
}
|
||||||
if t.io != nil {
|
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.Cancel()
|
||||||
t.io.Wait()
|
t.io.Wait()
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user