Repository navigation
Conversation
…uffer) FIONREAD writes back a C int (4 bytes) but bytes_to_read() handed fcntl.ioctl a 2-byte buffer and unpacked a short. Prior to Python 3.14 this was silently tolerated (ioctl copies into a 1024-byte static buffer and returns the first N bytes); 3.14 tightened fcntl.ioctl to raise SystemError on the buffer mismatch. Size the buffer to a full int and unpack accordingly. Same root cause as the TIOCGWINSZ fix in pyinvoke#1038 / pyinvoke#1040, which did not touch this code path. Fixes pyinvoke#1070.
Mukller
left a comment
There was a problem hiding this comment.
Verified as far as Windows allows (real CPython 3.14.5 via uv, editable install of the branch):
bytes_to_readguard paths work on 3.14/Windows: non-ttyStringIOand fileno-less objects return1without touching the ioctl branch.- The size claim is arithmetically exact: the old buffer
b" "is 2 bytes, while FIONREAD writes back a Cint(4 bytes) — so on 3.14's stricterfcntl.ioctlthis is a genuine buffer mismatch, not a style issue. The fix'sstruct.pack("i", 0)/unpack("i", ...)pair is symmetric at exactly 4 bytes. - The change mirrors the maintainer-accepted TIOCGWINSZ fixes (#1038/#1040) — same root cause class, same remedy, and per the PR description deliberately scoped away from that earlier patch.
Honest limits: the failing path requires a POSIX tty + fcntl, which I cannot exercise on Windows; their CI (Linux, including 3.14 jobs) is green, and the updated fake_ioctl in tests/runners.py now packs "i" consistently with the production code, so the mocked round-trip validates the new sizing.
Approving — this unblocks every POSIX user on 3.14, where the current release raises SystemError on any tty-driven read.
|
We tested this fix on Python 3.14 through our CLI’s Fabric/Paramiko interactive SSH path, and it resolves the input hang. Both Invoke 2.2.1 and 3.0.3 reproduce the failure; this PR passes. What’s the plan for merging this? Would a 2.x backport be possible, given Fabric 3.2.3’s invoke<3.0 constraint? |
canavandl
left a comment
There was a problem hiding this comment.
Resolved the issue in our CLI's Fabric/Paramikok interactive SSH path.
Fixes #1070.
Problem
On Python 3.14,
invoke.terminals.bytes_to_read()raisesSystemError: buffer overflow:FIONREADwrites back a Cint(4 bytes), but the buffer passed is only2 bytes. Before 3.14, CPython's
fcntl.ioctlcopied an immutable bufferinto a 1024-byte static scratch buffer and returned only the first
len(buf)bytes, so the undersized buffer was silently tolerated(and the 4-byte result truncated to 2). Python 3.14 tightened
fcntl.ioctlto raiseSystemErrorwhen the ioctl would write morebytes than the passed buffer holds (cpython#144206).
This is the same root cause as #1038, whose fix (#1040) only touched
the
TIOCGWINSZpath in_pty_size()and did not address this secondoccurrence in
bytes_to_read(). It bites anyRunner.run(pty=True)froma TTY-attached process on 3.14.
Fix
Size the buffer to a full native
intand unpack it as one:Tests
skip()pedterminals.bytes_to_read_.returns_FIONREAD_result_when_stream_is_a_ttyas a real regression test that drives the actual
ioctlagainst apty (a mock can't reproduce 3.14's buffer-size check). It asserts the
call does not raise, returns a non-negative int, and — to guard against
reintroducing the undersized buffer — that the buffer handed to
FIONREADisint-sized. Verified it fails withSystemError: buffer overflowon 3.14 without the fix and passes with it.FIONREADmock intests/runners.py(
reads_FIONREAD_bytes_from_stdin_when_fileno) tostruct.pack("i", …)to match.
Verified on Python 3.14.6 and 3.12.13 (
tests/terminals.py: 7 passed,1 skipped on both).
flake8clean.Note: the broader test suite still has unrelated Python 3.14 failures
(e.g.
tests/_util.py'sfakereadharness hardcodes fd numbers) that areindependent of this change and part of the wider 3.14-support effort
(#1015 / #1052); this PR is scoped to #1070 and introduces no new
failures (it turns 7 previously-failing FIONREAD-dependent tests green).
Thanks to @RemiDesgrange for the report and root-cause analysis in #1070.