Skip to content

Conversation

@gpshead
Copy link
Member

@gpshead gpshead commented May 30, 2025

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)
  • Added tests for memoryview inputs

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 <[email protected]>

pre-commit-whitespace-fixup
@gpshead gpshead force-pushed the subprocess-posix-input-memoryview branch from e1e8e52 to 6cd5c60 Compare May 30, 2025 18:42
@gpshead gpshead force-pushed the subprocess-posix-input-memoryview branch from 6cd5c60 to 7e1e75c Compare May 30, 2025 18:55
@gpshead gpshead requested a review from ZeroIntensity May 30, 2025 18:55
@gpshead gpshead enabled auto-merge (squash) November 29, 2025 04:01
@gpshead gpshead merged commit cc6bc4c into python:main Nov 29, 2025
45 of 46 checks passed
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 29, 2025
…ythonGH-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 <[email protected]>

* 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 <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Nov 29, 2025

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

@bedevere-app 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
…ythonGH-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 <[email protected]>

* 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 <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Nov 29, 2025

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

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Nov 29, 2025
@gpshead gpshead deleted the subprocess-posix-input-memoryview branch November 29, 2025 04:41
gpshead added a commit that referenced this pull request Nov 29, 2025
…H-134949) (#142062)

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)



* 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 <[email protected]>
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 FreeBSD Refleaks 3.14 (tier-3) has failed when building commit c1d3e25.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1800/builds/743) and take a look at the build logs.
  4. Check if the failure is related to this commit (c1d3e25) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1800/builds/743

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/buildbot/buildarea/3.14.ware-freebsd.refleak/build/Lib/test/support/__init__.py", line 847, in gc_collect
    gc.collect()
ResourceWarning: unclosed <socket.socket fd=9, family=2, type=1, proto=6, laddr=('127.0.0.1', 63223), raddr=('127.0.0.1', 63224)>
Task was destroyed but it is pending!
task: <Task pending name='Task-2667' coro=<BaseSelectorEventLoop._accept_connection2() done, defined at /buildbot/buildarea/3.14.ware-freebsd.refleak/build/Lib/asyncio/selector_events.py:217> wait_for=<Future pending cb=[Task.task_wakeup()]>>
Warning -- Unraisable exception
Exception ignored while calling deallocator <function _SelectorTransport.__del__ at 0x8467cbd10>:
Traceback (most recent call last):
  File "/buildbot/buildarea/3.14.ware-freebsd.refleak/build/Lib/asyncio/selector_events.py", line 873, in __del__
    _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
ResourceWarning: unclosed transport <_SelectorSocketTransport closing fd=9>
k


Traceback (most recent call last):
  File "/buildbot/buildarea/3.14.ware-freebsd.refleak/build/Lib/test/support/__init__.py", line 847, in gc_collect
    gc.collect()
ResourceWarning: unclosed <socket.socket fd=9, family=2, type=1, proto=6, laddr=('127.0.0.1', 12223), raddr=('127.0.0.1', 12224)>
Task was destroyed but it is pending!
task: <Task pending name='Task-320' coro=<BaseSelectorEventLoop._accept_connection2() done, defined at /buildbot/buildarea/3.14.ware-freebsd.refleak/build/Lib/asyncio/selector_events.py:217> wait_for=<Future pending cb=[Task.task_wakeup()]>>
Warning -- Unraisable exception
Exception ignored while calling deallocator <function _SelectorTransport.__del__ at 0x843167d10>:
Traceback (most recent call last):
  File "/buildbot/buildarea/3.14.ware-freebsd.refleak/build/Lib/asyncio/selector_events.py", line 873, in __del__
    _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
ResourceWarning: unclosed transport <_SelectorSocketTransport closing fd=9>
k

gpshead added a commit that referenced this pull request Nov 29, 2025
…H-134949) (#142063)

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)



* 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 <[email protected]>
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
…ythonGH-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 <[email protected]>

* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants