Skip to content

Commit dd2714d

Browse files
miss-islingtoncalvinrpclaude
authored
[3.15] gh-154836: Fix Popen.wait() with very large timeouts on the pidfd/kqueue wait paths (GH-154837) (#154891)
gh-154836: Fix Popen.wait() with very large timeouts on the pidfd/kqueue wait paths (GH-154837) The event-driven wait introduced by gh-83069 passes the caller's timeout unclamped to poll() / kqueue.control(), so values that do not fit the C timestamp conversion (float('inf'), sys.maxsize, 1e10, ...) raise OverflowError on Linux and, on macOS/BSD, a misleading "TypeError: timeout must be a real number or None" -- all of which worked on 3.14 and earlier. - Lib/subprocess.py: clamp each wait to _MAXIMUM_WAIT_TIMEOUT (24h, following asyncio's MAXIMUM_SELECT_TIMEOUT precedent) and loop until the real deadline in both _wait_pidfd() and _wait_kqueue(). - Modules/selectmodule.c: only rewrite the kqueue.control() timeout conversion failure into TypeError when the original exception IS a TypeError, exactly like the select()/poll()/devpoll()/epoll() sites, so OverflowError surfaces for out-of-range values. (cherry picked from commit 11d0da5) Co-authored-by: Calvin Prewitt <calvin@setout.dev> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 269638b commit dd2714d

5 files changed

Lines changed: 88 additions & 14 deletions

File tree

Lib/subprocess.py

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,12 @@ def _can_use_kqueue():
917917
_CAN_USE_PIDFD_OPEN = not _mswindows and _can_use_pidfd_open()
918918
_CAN_USE_KQUEUE = not _mswindows and _can_use_kqueue()
919919

920+
# Maximum timeout passed to poll() / kqueue.control() by Popen._wait():
921+
# very large values (e.g. timeout=float('inf')) overflow the C timestamp
922+
# conversion (gh-154836). Longer waits are performed in bounded slices
923+
# until the deadline, like asyncio's MAXIMUM_SELECT_TIMEOUT.
924+
_MAXIMUM_WAIT_TIMEOUT = 24 * 3600
925+
920926

921927
# These are primarily fail-safe knobs for negatives. A True value does not
922928
# guarantee the given libc/syscall API will be used.
@@ -2228,10 +2234,19 @@ def _wait_pidfd(self, timeout):
22282234
try:
22292235
poller = select.poll()
22302236
poller.register(pidfd, select.POLLIN)
2231-
events = poller.poll(timeout * 1000)
2232-
if not events:
2233-
raise TimeoutExpired(self.args, timeout)
2234-
return True
2237+
endtime = _time() + timeout
2238+
while True:
2239+
# Clamp very large timeouts (e.g. float('inf')):
2240+
# they overflow the C timestamp conversion in
2241+
# poll(). Wait in bounded slices until the real
2242+
# deadline (gh-154836).
2243+
delay = min(_deadline_remaining(endtime),
2244+
_MAXIMUM_WAIT_TIMEOUT)
2245+
events = poller.poll(max(delay, 0) * 1000)
2246+
if events:
2247+
return True
2248+
if _deadline_remaining(endtime) <= 0:
2249+
raise TimeoutExpired(self.args, timeout)
22352250
finally:
22362251
os.close(pidfd)
22372252

@@ -2252,14 +2267,26 @@ def _wait_kqueue(self, timeout):
22522267
flags=select.KQ_EV_ADD | select.KQ_EV_ONESHOT,
22532268
fflags=select.KQ_NOTE_EXIT,
22542269
)
2255-
try:
2256-
events = kq.control([kev], 1, timeout) # wait
2257-
except OSError:
2258-
return False
2259-
else:
2260-
if not events:
2270+
changelist = [kev]
2271+
endtime = _time() + timeout
2272+
while True:
2273+
# Clamp very large timeouts (e.g. float('inf')):
2274+
# they overflow the C timestamp conversion in
2275+
# kqueue.control(). Wait in bounded slices until
2276+
# the real deadline (gh-154836).
2277+
delay = min(_deadline_remaining(endtime),
2278+
_MAXIMUM_WAIT_TIMEOUT)
2279+
try:
2280+
events = kq.control(changelist, 1, max(delay, 0))
2281+
except OSError:
2282+
return False
2283+
if events:
2284+
return True
2285+
if _deadline_remaining(endtime) <= 0:
22612286
raise TimeoutExpired(self.args, timeout)
2262-
return True
2287+
# The kevent was registered by the first control()
2288+
# call; don't re-add it on later slices.
2289+
changelist = None
22632290
finally:
22642291
kq.close()
22652292

Lib/test/test_kqueue.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,20 @@ def test_create_queue(self):
2323
self.assertTrue(kq.closed)
2424
self.assertRaises(ValueError, kq.fileno)
2525

26+
def test_control_overflowing_timeout(self):
27+
# gh-154836: out-of-range timeouts must raise OverflowError,
28+
# not a (misleading) TypeError, like select(), poll() and
29+
# epoll() do.
30+
kq = select.kqueue()
31+
self.addCleanup(kq.close)
32+
for timeout in (1e300, float('inf'), 2**200):
33+
with self.subTest(timeout=timeout):
34+
with self.assertRaises(OverflowError):
35+
kq.control(None, 0, timeout)
36+
# Non-numbers still raise TypeError.
37+
with self.assertRaises(TypeError):
38+
kq.control(None, 0, "0.1")
39+
2640
def test_create_event(self):
2741
from operator import lt, le, gt, ge
2842

Lib/test/test_subprocess.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4290,5 +4290,31 @@ def test_fast_path_avoid_busy_loop(self):
42904290
self.assertEqual(p.wait(timeout=support.LONG_TIMEOUT), 0)
42914291
self.assertFalse(m.called)
42924292

4293+
@unittest.skipIf(mswindows, "requires the POSIX wait implementation")
4294+
def test_wait_huge_timeout(self):
4295+
# gh-154836: very large timeout values used to overflow the C
4296+
# timestamp conversion in poll() / kqueue.control() and raise
4297+
# OverflowError / TypeError.
4298+
for timeout in (10**10, sys.maxsize, float('inf')):
4299+
with self.subTest(timeout=timeout):
4300+
p = subprocess.Popen(ZERO_RETURN_CMD)
4301+
self.assertEqual(p.wait(timeout=timeout), 0)
4302+
4303+
@unittest.skipIf(mswindows, "requires the POSIX wait implementation")
4304+
def test_run_huge_timeout(self):
4305+
# gh-154836: same as test_wait_huge_timeout, via the
4306+
# subprocess.run() / communicate() code path.
4307+
cp = subprocess.run(ZERO_RETURN_CMD, timeout=1e10)
4308+
self.assertEqual(cp.returncode, 0)
4309+
4310+
@unittest.skipIf(mswindows, "requires the POSIX wait implementation")
4311+
def test_wait_slices_do_not_expire_early(self):
4312+
# A clamped wait slice must not raise TimeoutExpired before the
4313+
# real deadline: with a tiny slice limit, a process that
4314+
# outlives many slices must still be waited for successfully.
4315+
with mock.patch.object(subprocess, "_MAXIMUM_WAIT_TIMEOUT", 0.01):
4316+
p = subprocess.Popen(self.COMMAND) # sleeps 0.3s
4317+
self.assertEqual(p.wait(timeout=support.SHORT_TIMEOUT), 0)
4318+
42934319
if __name__ == "__main__":
42944320
unittest.main()
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix :meth:`subprocess.Popen.wait` raising :exc:`TypeError` (macOS and other
2+
BSDs) or :exc:`OverflowError` (Linux) for very large *timeout* values such
3+
as ``float('inf')``, a 3.15 regression in the new event-driven wait. Also
4+
fix :meth:`select.kqueue.control` masking :exc:`OverflowError` for
5+
out-of-range timeouts as :exc:`TypeError`.

Modules/selectmodule.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2365,9 +2365,11 @@ select_kqueue_control_impl(kqueue_queue_Object *self, PyObject *changelist,
23652365
else {
23662366
if (_PyTime_FromSecondsObject(&timeout,
23672367
otimeout, _PyTime_ROUND_TIMEOUT) < 0) {
2368-
PyErr_Format(PyExc_TypeError,
2369-
"timeout must be a real number or None, not %T",
2370-
otimeout);
2368+
if (PyErr_ExceptionMatches(PyExc_TypeError)) {
2369+
PyErr_Format(PyExc_TypeError,
2370+
"timeout must be a real number or None, not %T",
2371+
otimeout);
2372+
}
23712373
return NULL;
23722374
}
23732375

0 commit comments

Comments
 (0)