Skip to content

Commit 9f53a8e

Browse files
gpsheadclaude
andcommitted
Refactor POSIX communicate I/O into shared _communicate_io_posix()
Extract the core selector-based I/O loop into a new _communicate_io_posix() function that is shared by both _communicate_streams_posix() (used by run_pipeline) and Popen._communicate() (used by Popen.communicate). The new function: - Takes a pre-configured selector and output buffers - Supports resume via input_offset parameter (for Popen timeout retry) - Returns (new_offset, completed) instead of raising TimeoutExpired - Does not close streams (caller decides based on use case) This reduces code duplication and ensures both APIs use the same well-tested I/O multiplexing logic. Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 3c28ed6 commit 9f53a8e

File tree

1 file changed

+122
-94
lines changed

1 file changed

+122
-94
lines changed

Lib/subprocess.py

Lines changed: 122 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -320,14 +320,84 @@ def _cleanup():
320320
DEVNULL = -3
321321

322322

323-
# Helper function for multiplexed I/O, used by run_pipeline()
323+
# Helper function for multiplexed I/O
324324
def _remaining_time_helper(endtime):
325325
"""Calculate remaining time until deadline."""
326326
if endtime is None:
327327
return None
328328
return endtime - _time()
329329

330330

331+
def _communicate_io_posix(selector, stdin, input_view, input_offset,
332+
output_buffers, endtime):
333+
"""
334+
Low-level POSIX I/O multiplexing loop.
335+
336+
This is the common core used by both _communicate_streams() and
337+
Popen._communicate(). It handles the select loop for reading/writing
338+
but does not manage stream lifecycle or raise timeout exceptions.
339+
340+
Args:
341+
selector: A _PopenSelector with streams already registered
342+
stdin: Writable file object for input, or None
343+
input_view: memoryview of input bytes, or None
344+
input_offset: Starting offset into input_view (for resume support)
345+
output_buffers: Dict {file_object: list} to append read chunks to
346+
endtime: Deadline timestamp, or None for no timeout
347+
348+
Returns:
349+
(new_input_offset, completed)
350+
- new_input_offset: How many bytes of input were written
351+
- completed: True if all I/O finished, False if timed out
352+
353+
Note:
354+
- Does NOT close any streams (caller decides)
355+
- Does NOT raise TimeoutExpired (caller handles)
356+
- Appends to output_buffers lists in place
357+
"""
358+
stdin_fd = stdin.fileno() if stdin else None
359+
360+
while selector.get_map():
361+
remaining = _remaining_time_helper(endtime)
362+
if remaining is not None and remaining < 0:
363+
return (input_offset, False) # Timed out
364+
365+
ready = selector.select(remaining)
366+
367+
# Check timeout after select (may have woken spuriously)
368+
if endtime is not None and _time() > endtime:
369+
return (input_offset, False) # Timed out
370+
371+
for key, events in ready:
372+
if key.fd == stdin_fd:
373+
# Write chunk to stdin
374+
chunk = input_view[input_offset:input_offset + _PIPE_BUF]
375+
try:
376+
input_offset += os.write(key.fd, chunk)
377+
except BrokenPipeError:
378+
selector.unregister(key.fd)
379+
try:
380+
stdin.close()
381+
except BrokenPipeError:
382+
pass
383+
else:
384+
if input_offset >= len(input_view):
385+
selector.unregister(key.fd)
386+
try:
387+
stdin.close()
388+
except BrokenPipeError:
389+
pass
390+
elif key.fileobj in output_buffers:
391+
# Read chunk from output stream
392+
data = os.read(key.fd, 32768)
393+
if not data:
394+
selector.unregister(key.fileobj)
395+
else:
396+
output_buffers[key.fileobj].append(data)
397+
398+
return (input_offset, True) # Completed
399+
400+
331401
def _communicate_streams(stdin=None, input_data=None, read_streams=None,
332402
timeout=None, cmd_for_timeout=None):
333403
"""
@@ -426,86 +496,46 @@ def _communicate_streams_windows(stdin, input_data, read_streams,
426496
def _communicate_streams_posix(stdin, input_data, read_streams,
427497
endtime, orig_timeout, cmd_for_timeout):
428498
"""POSIX implementation using selectors."""
429-
# Build mapping of fd -> (file_object, chunks_list)
430-
fd_info = {}
431-
for stream in read_streams:
432-
fd_info[stream.fileno()] = (stream, [])
499+
# Build output buffers for each stream
500+
output_buffers = {stream: [] for stream in read_streams}
433501

434502
# Prepare stdin
435-
stdin_fd = None
436503
if stdin:
437504
try:
438505
stdin.flush()
439506
except BrokenPipeError:
440507
pass
441-
if input_data:
442-
stdin_fd = stdin.fileno()
443-
else:
508+
if not input_data:
444509
try:
445510
stdin.close()
446511
except BrokenPipeError:
447512
pass
513+
stdin = None # Don't register with selector
448514

449515
# Prepare input data
450-
input_offset = 0
451516
input_view = memoryview(input_data) if input_data else None
452517

453518
with _PopenSelector() as selector:
454-
if stdin_fd is not None and input_data:
455-
selector.register(stdin_fd, selectors.EVENT_WRITE)
456-
for fd in fd_info:
457-
selector.register(fd, selectors.EVENT_READ)
458-
459-
while selector.get_map():
460-
remaining = _remaining_time_helper(endtime)
461-
if remaining is not None and remaining < 0:
462-
# Timed out - collect partial results
463-
results = {stream: b''.join(chunks)
464-
for fd, (stream, chunks) in fd_info.items()}
465-
raise TimeoutExpired(
466-
cmd_for_timeout, orig_timeout,
467-
output=results.get(read_streams[0]) if read_streams else None)
468-
469-
ready = selector.select(remaining)
470-
471-
# Check timeout after select
472-
if endtime is not None and _time() > endtime:
473-
results = {stream: b''.join(chunks)
474-
for fd, (stream, chunks) in fd_info.items()}
475-
raise TimeoutExpired(
476-
cmd_for_timeout, orig_timeout,
477-
output=results.get(read_streams[0]) if read_streams else None)
478-
479-
for key, events in ready:
480-
if key.fd == stdin_fd:
481-
# Write chunk to stdin
482-
chunk = input_view[input_offset:input_offset + _PIPE_BUF]
483-
try:
484-
input_offset += os.write(key.fd, chunk)
485-
except BrokenPipeError:
486-
selector.unregister(key.fd)
487-
try:
488-
stdin.close()
489-
except BrokenPipeError:
490-
pass
491-
else:
492-
if input_offset >= len(input_data):
493-
selector.unregister(key.fd)
494-
try:
495-
stdin.close()
496-
except BrokenPipeError:
497-
pass
498-
elif key.fd in fd_info:
499-
# Read chunk from output stream
500-
data = os.read(key.fd, 32768)
501-
if not data:
502-
selector.unregister(key.fd)
503-
else:
504-
fd_info[key.fd][1].append(data)
519+
if stdin and input_data:
520+
selector.register(stdin, selectors.EVENT_WRITE)
521+
for stream in read_streams:
522+
selector.register(stream, selectors.EVENT_READ)
523+
524+
# Run the common I/O loop
525+
_, completed = _communicate_io_posix(
526+
selector, stdin, input_view, 0, output_buffers, endtime)
527+
528+
if not completed:
529+
# Timed out - collect partial results
530+
results = {stream: b''.join(chunks)
531+
for stream, chunks in output_buffers.items()}
532+
raise TimeoutExpired(
533+
cmd_for_timeout, orig_timeout,
534+
output=results.get(read_streams[0]) if read_streams else None)
505535

506536
# Build results and close all file objects
507537
results = {}
508-
for fd, (stream, chunks) in fd_info.items():
538+
for stream, chunks in output_buffers.items():
509539
results[stream] = b''.join(chunks)
510540
try:
511541
stream.close()
@@ -2633,6 +2663,10 @@ def _communicate(self, input, endtime, orig_timeout):
26332663
input_view = memoryview(self._input)
26342664
else:
26352665
input_view = self._input.cast("b") # byte input required
2666+
input_offset = self._input_offset
2667+
else:
2668+
input_view = None
2669+
input_offset = 0
26362670

26372671
with _PopenSelector() as selector:
26382672
if self.stdin and not self.stdin.closed and self._input:
@@ -2642,38 +2676,32 @@ def _communicate(self, input, endtime, orig_timeout):
26422676
if self.stderr and not self.stderr.closed:
26432677
selector.register(self.stderr, selectors.EVENT_READ)
26442678

2645-
while selector.get_map():
2646-
timeout = self._remaining_time(endtime)
2647-
if timeout is not None and timeout < 0:
2648-
self._check_timeout(endtime, orig_timeout,
2649-
stdout, stderr,
2650-
skip_check_and_raise=True)
2651-
raise RuntimeError( # Impossible :)
2652-
'_check_timeout(..., skip_check_and_raise=True) '
2653-
'failed to raise TimeoutExpired.')
2654-
2655-
ready = selector.select(timeout)
2656-
self._check_timeout(endtime, orig_timeout, stdout, stderr)
2657-
2658-
for key, events in ready:
2659-
if key.fileobj is self.stdin:
2660-
chunk = input_view[self._input_offset :
2661-
self._input_offset + _PIPE_BUF]
2662-
try:
2663-
self._input_offset += os.write(key.fd, chunk)
2664-
except BrokenPipeError:
2665-
selector.unregister(key.fileobj)
2666-
key.fileobj.close()
2667-
else:
2668-
if self._input_offset >= len(input_view):
2669-
selector.unregister(key.fileobj)
2670-
key.fileobj.close()
2671-
elif key.fileobj in (self.stdout, self.stderr):
2672-
data = os.read(key.fd, 32768)
2673-
if not data:
2674-
selector.unregister(key.fileobj)
2675-
key.fileobj.close()
2676-
self._fileobj2output[key.fileobj].append(data)
2679+
# Use the common I/O loop (supports resume via _input_offset)
2680+
stdin_to_write = (self.stdin if self.stdin and self._input
2681+
and not self.stdin.closed else None)
2682+
new_offset, completed = _communicate_io_posix(
2683+
selector,
2684+
stdin_to_write,
2685+
input_view,
2686+
input_offset,
2687+
self._fileobj2output,
2688+
endtime)
2689+
if self._input:
2690+
self._input_offset = new_offset
2691+
2692+
if not completed:
2693+
self._check_timeout(endtime, orig_timeout, stdout, stderr,
2694+
skip_check_and_raise=True)
2695+
raise RuntimeError( # Impossible :)
2696+
'_check_timeout(..., skip_check_and_raise=True) '
2697+
'failed to raise TimeoutExpired.')
2698+
2699+
# Close streams now that we're done reading
2700+
if self.stdout:
2701+
self.stdout.close()
2702+
if self.stderr:
2703+
self.stderr.close()
2704+
26772705
try:
26782706
self.wait(timeout=self._remaining_time(endtime))
26792707
except TimeoutExpired as exc:

0 commit comments

Comments
 (0)