Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
GH-134453: Fix subprocess memoryview input handling on POSIX (GH-134949)
Fix inconsistent subprocess.Popen.communicate() behavior between Windows
and POSIX when using memoryview objects with non-byte elements as input.

On POSIX systems, the code was incorrectly comparing bytes written against
element count instead of byte count, causing data truncation for large
inputs with non-byte element types.

Changes:
- Cast memoryview inputs to byte view when input is already a memoryview
- Fix progress tracking to use len(input_view) instead of len(self._input)
- Add comprehensive test coverage for memoryview inputs

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* old-man-yells-at-ReST
* Update 2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst
* assertIsNone review feedback
* fix memoryview_nonbytes test to fail without our fix on main, and have a nicer error.

Thanks to Peter Bierma @ZeroIntensity for the code review.
(cherry picked from commit cc6bc4c)

Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
  • Loading branch information
gpshead authored and miss-islington committed Nov 29, 2025
commit c9538b7d54c64c0d0468537c22f10ff5bd387ea8
7 changes: 5 additions & 2 deletions Lib/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -2105,7 +2105,10 @@ def _communicate(self, input, endtime, orig_timeout):
self._save_input(input)

if self._input:
input_view = memoryview(self._input)
if not isinstance(self._input, memoryview):
input_view = memoryview(self._input)
else:
input_view = self._input.cast("b") # byte input required

with _PopenSelector() as selector:
if self.stdin and input:
Expand Down Expand Up @@ -2141,7 +2144,7 @@ def _communicate(self, input, endtime, orig_timeout):
selector.unregister(key.fileobj)
key.fileobj.close()
else:
if self._input_offset >= len(self._input):
if self._input_offset >= len(input_view):
selector.unregister(key.fileobj)
key.fileobj.close()
elif key.fileobj in (self.stdout, self.stderr):
Expand Down
42 changes: 42 additions & 0 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,48 @@ def test_communicate(self):
self.assertEqual(stdout, b"banana")
self.assertEqual(stderr, b"pineapple")

def test_communicate_memoryview_input(self):
# Test memoryview input with byte elements
test_data = b"Hello, memoryview!"
mv = memoryview(test_data)
p = subprocess.Popen([sys.executable, "-c",
'import sys; sys.stdout.write(sys.stdin.read())'],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE)
self.addCleanup(p.stdout.close)
self.addCleanup(p.stdin.close)
(stdout, stderr) = p.communicate(mv)
self.assertEqual(stdout, test_data)
self.assertIsNone(stderr)

def test_communicate_memoryview_input_nonbyte(self):
# Test memoryview input with non-byte elements (e.g., int32)
# This tests the fix for gh-134453 where non-byte memoryviews
# had incorrect length tracking on POSIX
import array
# Create an array of 32-bit integers that's large enough to trigger
# the chunked writing behavior (> PIPE_BUF)
pipe_buf = getattr(select, 'PIPE_BUF', 512)
# Each 'i' element is 4 bytes, so we need more than pipe_buf/4 elements
# Add some extra to ensure we exceed the buffer size
num_elements = pipe_buf + 1
test_array = array.array('i', [0x64306f66 for _ in range(num_elements)])
expected_bytes = test_array.tobytes()
mv = memoryview(test_array)

p = subprocess.Popen([sys.executable, "-c",
'import sys; '
'data = sys.stdin.buffer.read(); '
'sys.stdout.buffer.write(data)'],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE)
self.addCleanup(p.stdout.close)
self.addCleanup(p.stdin.close)
(stdout, stderr) = p.communicate(mv)
self.assertEqual(stdout, expected_bytes,
msg=f"{len(stdout)=} =? {len(expected_bytes)=}")
self.assertIsNone(stderr)

def test_communicate_timeout(self):
p = subprocess.Popen([sys.executable, "-c",
'import sys,os,time;'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fixed :func:`subprocess.Popen.communicate` ``input=`` handling of :class:`memoryview`
instances that were non-byte shaped on POSIX platforms. Those are now properly
cast to a byte shaped view instead of truncating the input. Windows platforms
did not have this bug.