Skip to content

Commit a9cd017

Browse files
arr2036Arran Cudbard-Bell
andauthored
Fix SystemError in _DeferredCall.get() under GC pressure (#38355)
CPython builds the argument tuple incrementally via _PyTuple_Resize when unpacking a generator: f(*(gen)). _PyTuple_Resize guards that Py_REFCNT(v) == 1 before resizing a non-empty tuple. Under sustained allocation pressure a GC cycle can run between generator yields and temporarily increment the refcount on the partial tuple, causing _PyTuple_Resize to call PyErr_BadInternalCall(). Change the generator expression to a list comprehension. CPython builds the list first and passes it to CALL_FUNCTION_EX, which takes the PySequence_Fast list path and never calls _PyTuple_Resize. Co-authored-by: Arran Cudbard-Bell <arr2036@inkbridge.io>
1 parent a619a58 commit a9cd017

2 files changed

Lines changed: 40 additions & 1 deletion

File tree

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1448,7 +1448,14 @@ def wait(self, timeout=None):
14481448

14491449
def get(self, timeout=None):
14501450
# type: (Optional[float]) -> T
1451-
return self._func(*(arg.get(timeout) for arg in self._args))
1451+
# List comprehension, not generator: *(gen) causes CPython to build the
1452+
# argument tuple incrementally via _PyTuple_Resize, which asserts
1453+
# Py_REFCNT(v)==1. A GC cycle between yields can increment that refcount,
1454+
# raising SystemError (Objects/tupleobject.c:927). See
1455+
# https://github.com/python/cpython/issues/127058 (fixed in 3.14.0a3+:
1456+
# https://github.com/python/cpython/commit/5a23994). *[list] allocates the
1457+
# tuple once at its final size, avoiding the resize entirely.
1458+
return self._func(*[arg.get(timeout) for arg in self._args])
14521459

14531460
def set(self, value):
14541461
# type: (T) -> _Future[T]

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,38 @@ def testShortIdAssignment(self):
704704
% case.info)
705705

706706

707+
class DeferredCallTest(unittest.TestCase):
708+
"""Tests for _DeferredCall.get()."""
709+
def test_get_single_arg(self):
710+
f = sdk_worker._Future().set(42)
711+
call = sdk_worker._DeferredCall(lambda x: x, f)
712+
self.assertEqual(call.get(), 42)
713+
714+
def test_get_multiple_args(self):
715+
futures = [sdk_worker._Future().set(i) for i in range(5)]
716+
call = sdk_worker._DeferredCall(lambda *args: sum(args), *futures)
717+
self.assertEqual(call.get(), sum(range(5)))
718+
719+
def test_get_non_future_args_are_wrapped(self):
720+
# __init__ wraps non-Future values in _Future().set(v); get() must work.
721+
call = sdk_worker._DeferredCall(lambda x, y: x * y, 3, 7)
722+
self.assertEqual(call.get(), 21)
723+
724+
def test_get_mixed_future_and_value_args(self):
725+
a = sdk_worker._Future().set(10)
726+
call = sdk_worker._DeferredCall(lambda x, y: x + y, a, 5)
727+
self.assertEqual(call.get(), 15)
728+
729+
def test_get_zero_args(self):
730+
call = sdk_worker._DeferredCall(lambda: 99)
731+
self.assertEqual(call.get(), 99)
732+
733+
def test_get_preserves_return_value_type(self):
734+
f = sdk_worker._Future().set({'key': 'val'})
735+
call = sdk_worker._DeferredCall(lambda d: d, f)
736+
self.assertEqual(call.get(), {'key': 'val'})
737+
738+
707739
def monitoringInfoMetadata(info):
708740
return {
709741
descriptor.name: value

0 commit comments

Comments
 (0)