Some additional info from my thinking/digging on this: we have information "loss" of sorts between container create (where we know whether FIFOs are set by the caller versus defaulted or our own "constructors" are used) and delete/close; I don't see any way to connect these information "dots" unless we change the API to return additional details which would then allow for more nuanced approach to the close handler for FIFOs. If anyone else has better ideas, would love any insight.

Build succeeded.

• containerd-build-arm64 : SUCCESS in 3m 47s (non-voting)

Codecov Report

Merging #4123 into master will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #4123      +/-   ##
==========================================
- Coverage   42.43%   42.40%   -0.03%     
==========================================
  Files         129      130       +1     
  Lines       14875    14884       +9     
==========================================
  Hits         6312     6312              
- Misses       7644     7653       +9     
  Partials      919      919              

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0d4208...78ab1d1. Read the comment docs.

i'll look at this today

Thanks @crosbymichael! To be clear about my comment above, at create time we have a "richer" understanding of FIFOs; by the time we get to the loadFifos stage we only have 3 filepath strings from the task response. So, it seems to me any "closers" set up at the front end are all lost at this point if you started a detached task.

so this getFifoDir is only used to delete the fifos whenever you load a task and attach the IO.

I would almost say we defer this closer to be an opt that the client has to pass because they would know that type of IO they are connecting to. we could also just delete the individual files but we need a check to ensure that it is a fifo and not another type of file.

Maybe this is the best solution for now:

for f in files {
if f.type == fifo {
remove(f)
remove(dir(f)) // if there are still files in there remove will fail and it should be safe.
// if we get to the end and its empty then the dir gets removed
}

}

PTAL @crosbymichael

Build succeeded.

• containerd-build-arm64 : SUCCESS in 8m 01s (non-voting)

Build succeeded.

• containerd-build-arm64 : SUCCESS in 3m 45s (non-voting)

Build succeeded.

• containerd-build-arm64 : SUCCESS in 4m 20s (non-voting)

Not sure how the Travis run got disconnected from the PR check status: https://travis-ci.org/github/containerd/containerd/builds/666909149

LGTM

Most pleased to see this. Thanks @estesp !

I assume this will be in the next release, either 1.3.4 or 1.4.0? Is there an ETA on it?

@deitch no ETA, but several issues have been gathering in the 1.3 branch so I expect we will want to make a plan for 1.3.4 soon. 1.4 early alpha/betas should be coming soonish.

Thanks Phil!