FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-87512: Fix `subprocess` using `timeout=` on Windows blocking with a large `input=` by gpshead · Pull Request #142058 · python/cpython · GitHub

Repository navigation

gh-87512: Fix subprocess using timeout= on Windows blocking with a large input= - #142058

Merged
gpshead merged 3 commits into
python:mainfrom
gpshead:claude/fix-subprocess-windows-timeout-01YQBh24PZt4w9vhdqBQi6uW
Nov 29, 2025
Merged

gpshead merged 3 commits into
python:mainfrom
gpshead:claude/fix-subprocess-windows-timeout-01YQBh24PZt4w9vhdqBQi6uW

Conversation

gpshead commented Nov 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

On Windows, Popen._communicate() previously wrote to stdin synchronously, which could block indefinitely if the subprocess didn't consume input= quickly and the pipe buffer filled up. The timeout= parameter was only checked when joining the reader threads, not during the stdin write.

This change moves the Windows stdin writing to a background thread (similar to how stdout/stderr are read in threads), allowing the timeout to be properly enforced. If timeout expires, TimeoutExpired is raised promptly and the writer thread continues in the background. Subsequent calls to communicate() will join the existing writer thread.

Test driven development

I first added test_communicate_timeout_large_input to verify that TimeoutExpired is raised promptly when communicate() is called with large input and a timeout, even when the subprocess doesn't consume stdin quickly.

This test already passed on POSIX (where select() is used) but failed on Windows where the stdin write blocks without checking the timeout.

gpshead self-assigned this Nov 29, 2025
gpshead added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir OS-windows topic-subprocess Subprocess issues. labels Nov 29, 2025

This comment was marked as resolved.

gpshead force-pushed the claude/fix-subprocess-windows-timeout-01YQBh24PZt4w9vhdqBQi6uW branch 3 times, most recently from 87c0ac5 to bdc28cc Compare November 29, 2025 02:14
Add test_communicate_timeout_large_input to verify that TimeoutExpired
is raised promptly when communicate() is called with large input and
a timeout, even when the subprocess doesn't consume stdin quickly.

This test currently passes on POSIX (where select() is used) but is
expected to fail on Windows where the stdin write blocks without
checking the timeout.

The test sends 128KB of input (larger than typical pipe buffers) to
a subprocess that sleeps for 3 seconds before reading. With a 0.5s
timeout, TimeoutExpired should be raised within a couple seconds, not
after the subprocess finishes sleeping.

After timeout, the test verifies that input continuation works by
calling communicate() again and checking all data was received
(testing the fix from pythongh-141473).

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
gpshead force-pushed the claude/fix-subprocess-windows-timeout-01YQBh24PZt4w9vhdqBQi6uW branch from bdc28cc to 9b7fe29 Compare November 29, 2025 02:16
gpshead changed the title Add regression test for subprocess timeout during large input write gh-87512: subprocess using timeout= on Windows can block when a large input= is supplied Nov 29, 2025
gpshead changed the title gh-87512: subprocess using timeout= on Windows can block when a large input= is supplied gh-87512: Fix subprocess using timeout= on Windows blocking with a large input= Nov 29, 2025
gpshead force-pushed the claude/fix-subprocess-windows-timeout-01YQBh24PZt4w9vhdqBQi6uW branch from f05d0aa to 7d55f53 Compare November 29, 2025 02:48
gpshead and others added 2 commits November 29, 2025 03:07
…arge stdin

On Windows, Popen._communicate() previously wrote to stdin synchronously,
which could block indefinitely if the subprocess didn't consume input
quickly and the pipe buffer filled up. The timeout parameter was only
checked when joining the reader threads, not during the stdin write.

This change moves the stdin writing to a background thread (similar to
how stdout/stderr are read in threads), allowing the timeout to be
properly enforced. If timeout expires, TimeoutExpired is raised promptly
and the writer thread continues in the background. Subsequent calls to
communicate() will join the existing writer thread.

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
gpshead force-pushed the claude/fix-subprocess-windows-timeout-01YQBh24PZt4w9vhdqBQi6uW branch from 20d48a9 to 908fd43 Compare November 29, 2025 03:08
gpshead marked this pull request as ready for review November 29, 2025 04:47
gpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 29, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 908fd43 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F142058%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 29, 2025
gpshead merged commit 5b1862b into python:main Nov 29, 2025
99 of 110 checks passed
gpshead deleted the claude/fix-subprocess-windows-timeout-01YQBh24PZt4w9vhdqBQi6uW branch November 29, 2025 06:07
gpshead added needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels Nov 29, 2025

Copy link
Copy Markdown

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

Copy link
Copy Markdown

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 29, 2025
… with a large `input=` (pythonGH-142058)

On Windows, Popen._communicate() previously wrote to stdin synchronously, which could block indefinitely if the subprocess didn't consume input= quickly and the pipe buffer filled up. The timeout= parameter was only checked when joining the reader threads, not during the stdin write.

This change moves the Windows stdin writing to a background thread (similar to how stdout/stderr are read in threads), allowing the timeout to be properly enforced. If timeout expires, TimeoutExpired is raised promptly and the writer thread continues in the background. Subsequent calls to communicate() will join the existing writer thread.

Adds test_communicate_timeout_large_input to verify that TimeoutExpired is raised promptly when communicate() is called with large input and a timeout, even when the subprocess doesn't consume stdin quickly.

This test already passed on POSIX (where select() is used) but failed on Windows where the stdin write blocks without checking the timeout.
(cherry picked from commit 5b1862b)

Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

bedevere-app Bot commented Nov 29, 2025

Copy link
Copy Markdown

GH-142068 is a backport of this pull request to the 3.14 branch.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Nov 29, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 29, 2025
… with a large `input=` (pythonGH-142058)

On Windows, Popen._communicate() previously wrote to stdin synchronously, which could block indefinitely if the subprocess didn't consume input= quickly and the pipe buffer filled up. The timeout= parameter was only checked when joining the reader threads, not during the stdin write.

This change moves the Windows stdin writing to a background thread (similar to how stdout/stderr are read in threads), allowing the timeout to be properly enforced. If timeout expires, TimeoutExpired is raised promptly and the writer thread continues in the background. Subsequent calls to communicate() will join the existing writer thread.

Adds test_communicate_timeout_large_input to verify that TimeoutExpired is raised promptly when communicate() is called with large input and a timeout, even when the subprocess doesn't consume stdin quickly.

This test already passed on POSIX (where select() is used) but failed on Windows where the stdin write blocks without checking the timeout.
(cherry picked from commit 5b1862b)

Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

bedevere-app Bot commented Nov 29, 2025

Copy link
Copy Markdown

GH-142069 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 only security fixes label Nov 29, 2025
gpshead added a commit that referenced this pull request Nov 29, 2025
…g with a large `input=` (GH-142058) (#142068)

gh-87512: Fix `subprocess` using `timeout=` on Windows blocking with a large `input=` (GH-142058)

On Windows, Popen._communicate() previously wrote to stdin synchronously, which could block indefinitely if the subprocess didn't consume input= quickly and the pipe buffer filled up. The timeout= parameter was only checked when joining the reader threads, not during the stdin write.

This change moves the Windows stdin writing to a background thread (similar to how stdout/stderr are read in threads), allowing the timeout to be properly enforced. If timeout expires, TimeoutExpired is raised promptly and the writer thread continues in the background. Subsequent calls to communicate() will join the existing writer thread.

Adds test_communicate_timeout_large_input to verify that TimeoutExpired is raised promptly when communicate() is called with large input and a timeout, even when the subprocess doesn't consume stdin quickly.

This test already passed on POSIX (where select() is used) but failed on Windows where the stdin write blocks without checking the timeout.
(cherry picked from commit 5b1862b)

Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
gpshead added a commit that referenced this pull request Nov 29, 2025
…g with a large `input=` (GH-142058) (#142069)

gh-87512: Fix `subprocess` using `timeout=` on Windows blocking with a large `input=` (GH-142058)

On Windows, Popen._communicate() previously wrote to stdin synchronously, which could block indefinitely if the subprocess didn't consume input= quickly and the pipe buffer filled up. The timeout= parameter was only checked when joining the reader threads, not during the stdin write.

This change moves the Windows stdin writing to a background thread (similar to how stdout/stderr are read in threads), allowing the timeout to be properly enforced. If timeout expires, TimeoutExpired is raised promptly and the writer thread continues in the background. Subsequent calls to communicate() will join the existing writer thread.

Adds test_communicate_timeout_large_input to verify that TimeoutExpired is raised promptly when communicate() is called with large input and a timeout, even when the subprocess doesn't consume stdin quickly.

This test already passed on POSIX (where select() is used) but failed on Windows where the stdin write blocks without checking the timeout.
(cherry picked from commit 5b1862b)

Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
… with a large `input=` (pythonGH-142058)

On Windows, Popen._communicate() previously wrote to stdin synchronously, which could block indefinitely if the subprocess didn't consume input= quickly and the pipe buffer filled up. The timeout= parameter was only checked when joining the reader threads, not during the stdin write.

This change moves the Windows stdin writing to a background thread (similar to how stdout/stderr are read in threads), allowing the timeout to be properly enforced. If timeout expires, TimeoutExpired is raised promptly and the writer thread continues in the background. Subsequent calls to communicate() will join the existing writer thread.

Adds test_communicate_timeout_large_input to verify that TimeoutExpired is raised promptly when communicate() is called with large input and a timeout, even when the subprocess doesn't consume stdin quickly.

This test already passed on POSIX (where select() is used) but failed on Windows where the stdin write blocks without checking the timeout.

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OS-windows tests Tests in the Lib/test dir topic-subprocess Subprocess issues. type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL