tokio-uring
tokio-uring copied to clipboard
tokio-rs
TcpListener.accept is not cancellation safe, an open fd can be leaked
It took some finagling but I was able to create a scenario where the Future created by a TcpListener.accept call was partially polled, but not to completion. In select! parlance, a test I wrote shows the Future is not cancellation safe because the side effect was that file descriptors were being assigned by the kernel and never closed (until the process was terminated).
This is what I had expected but only after seeing the more obvious case of the multi accept Stream being dropped while the kernel races to create file descriptors that the user land will never get to recognize - they are put into the cq and read out as cqe's that were being ignored.
So some cleanup function should be definable for the singleshot accept case and the multishot accept case where the ignored lifetime variant still has a way to reclaim resources in a synchronous way, based on the type of operation that is now being ignored.
I was going to work on the multishot case for this fd leak but after realizing it can affect the singleshot case too, and getting an example binary that can reproduce the singleshot case, planning a solution first for the more general singleshot case seems prudent.
I agree. I think the cancellation of the future itself, with respect to the slab and what not is working but here is a case where we would need some cleanup function to be called, to avoid leaking a resource that the kernel (and libc?) have given us.
Want to propose something?
I'd like to do so soon, but I'm flying again today to visit family and won't be back at my place until Sunday night. I might be able to do something between then and now but probably not.
I'm wondering if an API where we build the future for the op, but before calling await, we hydrate it somehow with a cleanup handler. For this particular case, some type of handler that takes the result and closes the file descriptor that can be gleaned from it.
This is what I've been thinking about as well. I kinda wanna do this with my upcoming (soon:tm:) draft PR of new "builder-style" op semantics. I'd like to generify a few things like this.