IO.Cancel() becomes a no-op if the FIFOs are already opened

[davidhsingyuchen]

Description

The comment of IO.Cancel() says "Cancel aborts all current io operations", and I suppose it means stopping the goroutines in copyIO (e.g., the FIFOs will remain there, but the content will stop being copied to the provided IO streams). However, cio.Cancel() calls the cancel() function associated with the context that was passed to fifo.OpenFifo, which comment says "Context can be used to cancel this function until open(2) has not returned.", which means that IO.Cancel() becomes a no-op if the FIFOs are already opened.

Steps to reproduce the issue

t, _ := container.NewTask(ctx, cio.NewCreator(cio.WithStdio))
// After the FIFOs are created
t.IO().Cancel()
t.IO().Wait() // this blocks forever

Describe the results you received and expected

Received

IO.Cancel() becomes a no-op if the FIFOs are already opened, and IO.Wait() blocks forever as a result.

Expected

IO.Cancel() should close (not remove) the FIFOs, and IO.Wait() can be used to detect when that's done.

What version of containerd are you using?

containerd github.com/containerd/containerd v1.6.19 1e1ea6e

Any other relevant information

For more context, I'm trying to implement --detach-keys for nerdctl (containerd/nerdctl/pull/2128).

Show configuration if it is related to CRI plugin.

No response

[fahedouch]

Hi @davidhsingyuchen,

I rechecked containerd base code and I think we misunderstood some some functionalities:

// Cancel aborts all current io operations.
Cancel()

the above comment is missing a very important detail. Looking at the below comment

// OpenFifo opens a fifo. Returns io.ReadWriteCloser.
// Context can be used to cancel this function until open(2) has not returned.

=> cancel() aborts all current io operations until open(2) has not returned.

When container.NewTask(ctx, cio.NewCreator(cio.WithStdio)) returns, the open(2) may have already returned (fifo is opened). So the cancel() may have no effect.

I believe we have to cancel io then close fifos before waiting for io

t, _ := container.NewTask(ctx, cio.NewCreator(cio.WithStdio))
// After the FIFOs are created an opened
t.IO().Cancel()
t.IO().Close()
t.IO().Wait() // this blocks forever

please see how container.NewTask handles io in case of error

[davidhsingyuchen]

I believe we have to cancel io then close fifos before waiting for io

IO().Close() removes the FIFOs, which means that we cannot re-attach to it, while that's what we want. Similar to containerd/nerdctl#2128 (comment).

[fahedouch]

I believe we have to cancel io then close fifos before waiting for io

IO().Close() removes the FIFOs, which means that we cannot re-attach to it, while that's what we want. Similar to containerd/nerdctl#2128 (comment).

Ah Sorry! I forgot to check previous comments 😅