Skip to content

Commit d420f29

Browse files
gpsheadclaude
andcommitted
Fix _communicate_streams_windows to avoid blocking with large input
Move stdin writing to a background thread in _communicate_streams_windows to avoid blocking indefinitely when writing large input to a pipeline where the subprocess doesn't consume stdin quickly. This mirrors the fix made to Popen._communicate() for Windows in commit 5b1862b (gh-87512). Add test_pipeline_timeout_large_input to verify that TimeoutExpired is raised promptly when run_pipeline() is called with large input and a timeout, even when the first process is slow to consume stdin. Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 9f53a8e commit d420f29

File tree

2 files changed

+86
-18
lines changed

2 files changed

+86
-18
lines changed

Lib/subprocess.py

Lines changed: 53 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -443,11 +443,48 @@ def _reader_thread_func(fh, buffer):
443443
except OSError:
444444
buffer.append(b'')
445445

446+
def _writer_thread_func(fh, data, result):
447+
"""Thread function to write data to a file handle and close it."""
448+
try:
449+
if data:
450+
fh.write(data)
451+
except BrokenPipeError:
452+
pass
453+
except OSError as exc:
454+
if exc.errno != errno.EINVAL:
455+
result.append(exc)
456+
try:
457+
fh.close()
458+
except BrokenPipeError:
459+
pass
460+
except OSError as exc:
461+
if exc.errno != errno.EINVAL and not result:
462+
result.append(exc)
463+
446464
def _communicate_streams_windows(stdin, input_data, read_streams,
447465
endtime, orig_timeout, cmd_for_timeout):
448466
"""Windows implementation using threads."""
449467
threads = []
450468
buffers = {}
469+
writer_thread = None
470+
writer_result = []
471+
472+
# Start writer thread to send input to stdin
473+
if stdin and input_data:
474+
writer_thread = threading.Thread(
475+
target=_writer_thread_func,
476+
args=(stdin, input_data, writer_result))
477+
writer_thread.daemon = True
478+
writer_thread.start()
479+
elif stdin:
480+
# No input data, just close stdin
481+
try:
482+
stdin.close()
483+
except BrokenPipeError:
484+
pass
485+
except OSError as exc:
486+
if exc.errno != errno.EINVAL:
487+
raise
451488

452489
# Start reader threads for each stream
453490
for stream in read_streams:
@@ -458,25 +495,23 @@ def _communicate_streams_windows(stdin, input_data, read_streams,
458495
t.start()
459496
threads.append((stream, t))
460497

461-
# Write stdin
462-
if stdin and input_data:
463-
try:
464-
stdin.write(input_data)
465-
except BrokenPipeError:
466-
pass
467-
except OSError as exc:
468-
if exc.errno != errno.EINVAL:
469-
raise
470-
if stdin:
471-
try:
472-
stdin.close()
473-
except BrokenPipeError:
474-
pass
475-
except OSError as exc:
476-
if exc.errno != errno.EINVAL:
477-
raise
498+
# Join writer thread with timeout first
499+
if writer_thread is not None:
500+
remaining = _remaining_time_helper(endtime)
501+
if remaining is not None and remaining < 0:
502+
remaining = 0
503+
writer_thread.join(remaining)
504+
if writer_thread.is_alive():
505+
# Timed out during write - collect partial results
506+
results = {s: (b[0] if b else b'') for s, b in buffers.items()}
507+
raise TimeoutExpired(
508+
cmd_for_timeout, orig_timeout,
509+
output=results.get(read_streams[0]) if read_streams else None)
510+
# Check for write errors
511+
if writer_result:
512+
raise writer_result[0]
478513

479-
# Join threads with timeout
514+
# Join reader threads with timeout
480515
for stream, t in threads:
481516
remaining = _remaining_time_helper(endtime)
482517
if remaining is not None and remaining < 0:

Lib/test/test_subprocess.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2298,6 +2298,39 @@ def test_pipeline_large_data_with_stderr(self):
22982298
self.assertGreater(len(result.stderr), stderr_size)
22992299
self.assertEqual(result.returncodes, [0, 0])
23002300

2301+
def test_pipeline_timeout_large_input(self):
2302+
"""Test that timeout is enforced with large input to a slow pipeline.
2303+
2304+
This verifies that run_pipeline() doesn't block indefinitely when
2305+
writing large input to a pipeline where the first process is slow
2306+
to consume stdin. The timeout should be enforced promptly.
2307+
2308+
This is particularly important on Windows where stdin writing could
2309+
block without proper threading.
2310+
"""
2311+
# Input larger than typical pipe buffer (64KB)
2312+
input_data = 'x' * (128 * 1024)
2313+
2314+
start = time.monotonic()
2315+
with self.assertRaises(subprocess.TimeoutExpired):
2316+
subprocess.run_pipeline(
2317+
# First process sleeps before reading - simulates slow consumer
2318+
[sys.executable, '-c',
2319+
'import sys, time; time.sleep(30); print(sys.stdin.read())'],
2320+
[sys.executable, '-c',
2321+
'import sys; print(len(sys.stdin.read()))'],
2322+
input=input_data, capture_output=True, text=True, timeout=0.5
2323+
)
2324+
elapsed = time.monotonic() - start
2325+
2326+
# Timeout should occur close to the specified timeout value,
2327+
# not after waiting for the subprocess to finish sleeping.
2328+
# Allow generous margin for slow CI, but must be well under
2329+
# the subprocess sleep time.
2330+
self.assertLess(elapsed, 5.0,
2331+
f"TimeoutExpired raised after {elapsed:.2f}s; expected ~0.5s. "
2332+
"Input writing may have blocked without checking timeout.")
2333+
23012334

23022335
def _get_test_grp_name():
23032336
for name_group in ('staff', 'nogroup', 'grp', 'nobody', 'nfsnobody'):

0 commit comments

Comments
 (0)