GeezerVison

I'm not a Windows person so I'm mostly just going over this from a logical point of view and asking claude if these semantics make sense on Windows. I think this is workable but has flaws. So it still seems "better" (ie: landing it would be an improvement) but is not sufficient to fix the problem:

(claude's explanation below)

1. The cancel is one-shot and can miss. CancelSynchronousIo() only affects a thread that is inside ReadFile at that instant. If the reader is between PeekNamedPipe() and ReadFile(), or appending to the buffer, we get ERROR_NOT_FOUND (swallowed) and the thread then blocks in a read that nothing will cancel. In run() the following communicate() joins without a timeout, so we hang as before. The new tests can't hit this because the grandchild writes once and goes quiet; a grandchild that keeps writing would. I think this needs a "cancelled" flag that the reader loop checks before each read, with _cancel_io() repeating cancel + short join() until the thread exits.

2. A missed cancel in __exit__ leads to a read on a closed handle. The reader uses a raw handle captured once from get_osfhandle(). If the cancel misses, self.stdout.close() succeeds and the reader then calls ReadFile on a stale handle value: an unhandled OSError in the thread at best, a read from an unrelated reused handle at worst. The fix for 1 covers this too.

3. Could this use CancelIoEx(handle, NULL) instead? CancelSynchronousIo() is thread-targeted, so it cancels whatever synchronous I/O that thread happens to be doing (e.g. a finalizer running on it), and looking the thread up by thread.ident after is_alive() is open to thread ID reuse. My understanding is that CancelIoEx() also cancels synchronous I/O issued by other threads but is scoped to the pipe handle, which would avoid both problems and remove the need for OpenThread()/THREAD_TERMINATE. Is there a reason it doesn't work here? (It has the same miss race, so 1 still applies.)

4. Behaviour change: buffered data is dropped. fh.read() used to go through the BufferedReader/TextIOWrapper, so p.stdout.readline() followed by p.communicate() returned the rest. Reading the raw handle skips anything already buffered. POSIX already behaves this way, so this may be acceptable, but it should be a conscious decision and mentioned in the NEWS entry. [me: ugh!]

5. Is PeekNamedPipe() needed? ReadFile() on a byte-mode pipe returns as soon as any data is available, so ReadFile(handle, 32768) should give the same cancellable behaviour with half the syscalls, no peek-to-read window, and no 4K chunk cap on large outputs.

Smaller things:

• The stdin writer path is cancelled too but has no test (input= with a full pipe). It relies on the aborted write surfacing as EINVAL so _stdin_write() swallows it, and the self.stdin.close() after it may flush and block again.
• kill() then communicate() after a communicate(timeout=...) on a bare Popen (the pattern from the docs) still hangs, since only run() and __exit__ cancel. Fine if intentionally out of scope, but worth stating.
• OpenThread lacks a docstring, and there are no test_winapi tests for the new functions.

Points 3 and 5 are claude's recollection of CancelIoEx and pipe ReadFile semantics - that needs verification.