Skip to content

Commit 92af6ef

Browse files
committed
Address review comments: add CPython issue ref, trim test docstring
1 parent 6628a3d commit 92af6ef

2 files changed

Lines changed: 3 additions & 26 deletions

File tree

sdks/python/apache_beam/runners/worker/sdk_worker.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1457,7 +1457,8 @@ def get(self, timeout=None):
14571457
# List comprehension, not generator: *(gen) causes CPython to build the
14581458
# argument tuple incrementally via _PyTuple_Resize, which asserts
14591459
# Py_REFCNT(v)==1. A GC cycle between yields can increment that refcount,
1460-
# raising SystemError (Objects/tupleobject.c:927). *[list] allocates the
1460+
# raising SystemError (Objects/tupleobject.c:927). See cpython/issues/127058
1461+
# (fixed in 3.14: cpython@5a23994). *[list] allocates the
14611462
# tuple once at its final size, avoiding the resize entirely.
14621463
return self._func(*[arg.get(timeout) for arg in self._args])
14631464

sdks/python/apache_beam/runners/worker/sdk_worker_test.py

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -705,31 +705,7 @@ def testShortIdAssignment(self):
705705

706706

707707
class DeferredCallTest(unittest.TestCase):
708-
"""Tests for _DeferredCall.get().
709-
710-
Background: the original implementation used a generator expression in the
711-
argument unpack position:
712-
713-
return self._func(*(arg.get(timeout) for arg in self._args))
714-
715-
CPython builds the argument tuple incrementally via _PyTuple_Resize as it
716-
drains the generator. _PyTuple_Resize guards that Py_REFCNT(v) == 1 before
717-
resizing a non-empty tuple (Objects/tupleobject.c). Under sustained load,
718-
a GC cycle can run between generator yields and temporarily increment the
719-
refcount on the partially-built tuple, causing _PyTuple_Resize to call
720-
PyErr_BadInternalCall() and raise SystemError. This was observed in
721-
production workers (Python 3.11, Beam 2.73.0) at
722-
Objects/tupleobject.c:927.
723-
724-
The partial tuple is a C-level local inside PySequence_Tuple, so the race
725-
cannot be triggered deterministically from pure Python. The tests here
726-
verify correct behaviour; the crash itself requires CPython internal timing
727-
or a debug build to reproduce reliably.
728-
729-
Fix: change the generator to a list comprehension. CPython builds the list
730-
first and passes it to CALL_FUNCTION_EX, which calls PySequence_Fast on a
731-
list (a no-op path that does not call _PyTuple_Resize).
732-
"""
708+
"""Tests for _DeferredCall.get()."""
733709
def test_get_single_arg(self):
734710
f = sdk_worker._Future().set(42)
735711
call = sdk_worker._DeferredCall(lambda x: x, f)

0 commit comments

Comments
 (0)