NamedPipe permanently leaks HANDLEs

[1c7718e7]

Inside SelectorInner::drop(), mio does timeout=0 polls of IOCP in an attempt to reap all pending overlapped ops NamedPipes might have submited. This is needed to ensure that all references to named_pipe::Inner get released (which will then call CloseHandle on the pipe).
However, it's impossible to guarantee that all NamedPipe reads/writes will complete and get witnessed by the time mio::Poll gets dropped.
This leads to the underlying handle of NamedPipe never getting closed.

Test case to reproduce the bug

1c7718e7@94e6627

Reproduces on both Windows 11 and Wine 11.5.

Potential fix

1c7718e7@8df3ebc

This simply wraps the handle as Mutex<Option> and makes NamedPipe::drop synchronously CloseHandle it.
Wrapping it as std::sync::Weak and holding a strong Arc inside NamedPipe is would be another alternative.

[omaremadcc]

Isn't the expected behavior for the NamedPipe to not stop writing when dropped

impl Drop for NamedPipe {
    fn drop(&mut self) {
        // Cancel pending reads/connects, but don't cancel writes to ensure that
        // everything is flushed out.
        unsafe {
            if self.inner.connecting.load(SeqCst) {
                drop(cancel(&self.inner.handle, &self.inner.connect));
            }

            let io = self.inner.io.lock().unwrap();
            if let State::Pending(..) = io.read {
                drop(cancel(&self.inner.handle, &self.inner.read));
            }
        }
    }
}

[1c7718e7]

Yes, that what the comment says, and it is also the behavior of network sockets (due to the readiness model).
Mio tries to emulate that with named pipes, but doing that would have one of the following unfortunate consequences:
a) Poll::drop unavoidably leaks handles + Arc<Inner> + the data buffers (the current behavior)
b) Poll::drop is blocking (if it were to robustly wait for all overlappeds to complete)

Even if the list of all NamedPipes were somehow tracked under Poll and Poll::drop would close them, it still would have to leak data buffers, since those can only be freed once the operations complete, and that is blocking.

I think the best solution would be to change the semantics of NamedPipe slightly: writes to NamedPipe are guaranteed to flushed out only after std::io::Write::flush() returns Ok (the same way most buffered io impls work). The flush() impl would just need to check whether the inner.io.write state is pending or not (the current one just returns Ok unconditionally). I'm planning to make a PR for this, but I would like to hear some input first, considering the changes in semantics.

[Darksonn]

I agree that at the very least we must close the handle when dropping the Poll.

[ameyypawar]

Had a look at this. The leak traces back to the mem::forget(me.clone()) in schedule_read/maybe_schedule_write — that Arc<Inner> refcount only comes back when the completion fires, so if SelectorInner::drop's zero-timeout poll misses a still-pending op, Inner never drops and the handle's CloseHandle never runs.

@1c7718e7's Mutex<Option<Handle>> approach looks like a sound way to close the handle independently of that refcount. The real sticking point seems to be the write semantics — closing the handle on drop means in-flight writes may no longer flush, which is what the current "don't cancel writes" comment promises.

@Darksonn would you be open to that change in the write-flush guarantee? That feels like the thing blocking a PR here.