[limadog9]

Make Popen.communicate() handle already-closed pipes consistently across its optimized path, selector path, and Windows reader threads. Closed output pipes return the same empty bytes or string that the selector path already returned. Sending nonempty input to closed stdin raises ValueError before communication starts; calls without input still succeed.

Add regressions covering stdout and stderr, one or multiple pipes, binary and text modes, and calls with and without timeouts. Check that communication can still finish after rejected input. Set the existing broken-pipe mocks' closed state explicitly so they continue to exercise open pipes.

Fixes #131064.

Validation:

• The closed-output regression reproduces six failing cases on unmodified main. The stdin regression fails in all 16 subcases with the previous revision and passes with explicit validation.
• Full subprocess suite on a free-threaded debug build: 375 tests, 48 skipped.
• All 46 communicate tests also pass on a GIL debug build.
• Seven tests exercise the Windows communication methods using real POSIX pipes. Native Windows validation is left to CI.
• Ruff, NEWS lint, and patchcheck.

• Issue: Inconsistent Popen.communicate() behavior if stdin/stdout/stderr is closed PIPE #131064

[pekkaklarck]

As the submitter of the original issue this looks pretty good to me. I only looked at this on a mobile screen, didn't check are all code paths covered and didn't properly check what the added test does, though.

[limadog9]

@pekkaklarck Thanks for the review. I’ve updated and squashed the changes:

• Sending nonempty input to closed stdin now raises ValueError before communication starts. Calling without input still works, and the tests cover both cases.
• Changed the stdout/stderr expressions to the ordering you suggested.
• The reader thread now skips closed streams without appending anything. Joining the buffers afterward preserves empty bytes/strings for captured streams and None for uncaptured streams.

[limadog9]

@pekkaklarck Agreed, I’ve moved fh.close() inside the if not fh.closed block

[pekkaklarck]

It seems to me that these changes aren't needed. At this point stdout and stderr are either None or lists self._stdout_buff or self._stderr_buff, respectively. These lists earlier contained exactly one value, but now they can also be empty. The existing if stdout/if stderr conditions cover both the None case and then empty list case.

I consider the current code rather confusing, though, and would consider changing it to something like this:

stdout = None
stderr = None
if self.stdout:
    self.stdout.close()
    if self._stdout_buff:
        stdout = self._stdout_buff[0]
if self.stderr:
    self.stderr.close()
    if self._stderr_buff:
        stderr = self._stderr_buff[0]

return (stdout, stderr)