Skip to content

Commit 91bc1ae

Browse files
gh-83371: Fix deadlock when a Pool callback raises an exception
The exception killed the thread which handles results, so that the pool hung forever. It is now the result of the job and is raised by AsyncResult.get(), with the original error as its context. Co-authored-by: Sindri Guðmundsson <sindrigudmundsson@gmail.com>
1 parent c72ea53 commit 91bc1ae

3 files changed

Lines changed: 157 additions & 17 deletions

File tree

Lib/multiprocessing/pool.py

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,16 @@ def __enter__(self):
754754
def __exit__(self, exc_type, exc_val, exc_tb):
755755
self.terminate()
756756

757+
def _chain_context(exc, context):
758+
'Set context as the context of exc, avoiding a cycle.'
759+
seen = {id(context)}
760+
while exc is not None and id(exc) not in seen:
761+
seen.add(id(exc))
762+
if exc.__context__ is None:
763+
exc.__context__ = context
764+
return
765+
exc = exc.__context__
766+
757767
#
758768
# Class whose instances are returned by `Pool.apply_async()`
759769
#
@@ -791,13 +801,25 @@ def get(self, timeout=None):
791801

792802
def _set(self, i, obj):
793803
self._success, self._value = obj
794-
if self._callback and self._success:
795-
self._callback(self._value)
796-
if self._error_callback and not self._success:
797-
self._error_callback(self._value)
798-
self._event.set()
799-
del self._cache[self._job]
800-
self._pool = None
804+
try:
805+
if self._success:
806+
if self._callback:
807+
self._callback(self._value)
808+
else:
809+
if self._error_callback:
810+
self._error_callback(self._value)
811+
except BaseException as exc:
812+
# A failed callback becomes the result of the job. If it
813+
# propagated, it would kill the result handler thread.
814+
if not self._success:
815+
# do not lose the original error
816+
_chain_context(exc, self._value)
817+
self._success = False
818+
self._value = exc
819+
finally:
820+
self._event.set()
821+
del self._cache[self._job]
822+
self._pool = None
801823

802824
__class_getitem__ = classmethod(types.GenericAlias)
803825

@@ -828,23 +850,33 @@ def _set(self, i, success_result):
828850
if success and self._success:
829851
self._value[i*self._chunksize:(i+1)*self._chunksize] = result
830852
if self._number_left == 0:
831-
if self._callback:
832-
self._callback(self._value)
833-
del self._cache[self._job]
834-
self._event.set()
835-
self._pool = None
853+
try:
854+
if self._callback:
855+
self._callback(self._value)
856+
except BaseException as exc:
857+
self._success = False
858+
self._value = exc
859+
finally:
860+
del self._cache[self._job]
861+
self._event.set()
862+
self._pool = None
836863
else:
837864
if not success and self._success:
838865
# only store first exception
839866
self._success = False
840867
self._value = result
841868
if self._number_left == 0:
842869
# only consider the result ready once all jobs are done
843-
if self._error_callback:
844-
self._error_callback(self._value)
845-
del self._cache[self._job]
846-
self._event.set()
847-
self._pool = None
870+
try:
871+
if self._error_callback:
872+
self._error_callback(self._value)
873+
except BaseException as exc:
874+
_chain_context(exc, self._value)
875+
self._value = exc
876+
finally:
877+
del self._cache[self._job]
878+
self._event.set()
879+
self._pool = None
848880

849881
#
850882
# Class whose instances are returned by `Pool.imap()`

Lib/test/_test_multiprocessing.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3452,12 +3452,114 @@ def test_resource_warning(self):
34523452
pool = None
34533453
support.gc_collect()
34543454

3455+
class CallbackError(Exception): pass
3456+
3457+
class CallbackBaseException(BaseException): pass
3458+
34553459
def raising():
34563460
raise KeyError("key")
34573461

3462+
def raising_map(x):
3463+
raise KeyError("key")
3464+
3465+
def reraise(exc):
3466+
raise exc
3467+
3468+
def raise_with_context(exc):
3469+
try:
3470+
raise ZeroDivisionError
3471+
except ZeroDivisionError:
3472+
raise CallbackError('callback failed')
3473+
34583474
def unpickleable_result():
34593475
return lambda: 42
34603476

3477+
class _TestPoolCallbackErrors(BaseTestCase):
3478+
ALLOWED_TYPES = ('processes', )
3479+
3480+
@staticmethod
3481+
def _raise(value):
3482+
raise CallbackError('callback failed')
3483+
3484+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
3485+
def test_apply_async_callback_raises(self):
3486+
with multiprocessing.Pool(1) as p:
3487+
res = p.apply_async(sqr, (7,), callback=self._raise)
3488+
with self.assertRaises(CallbackError):
3489+
res.get(support.SHORT_TIMEOUT)
3490+
# the pool is still usable
3491+
self.assertEqual(p.apply(sqr, (3,)), 9)
3492+
self.assertTrue(p._result_handler.is_alive())
3493+
3494+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
3495+
def test_apply_async_callback_raises_base_exception(self):
3496+
def raise_base(value):
3497+
raise CallbackBaseException
3498+
with multiprocessing.Pool(1) as p:
3499+
res = p.apply_async(sqr, (7,), callback=raise_base)
3500+
with self.assertRaises(CallbackBaseException):
3501+
res.get(support.SHORT_TIMEOUT)
3502+
# the pool did not hang
3503+
self.assertEqual(p.apply(sqr, (3,)), 9)
3504+
3505+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
3506+
def test_apply_async_error_callback_raises(self):
3507+
with multiprocessing.Pool(1) as p:
3508+
res = p.apply_async(raising, error_callback=self._raise)
3509+
with self.assertRaises(CallbackError) as cm:
3510+
res.get(support.SHORT_TIMEOUT)
3511+
# the original error is not lost
3512+
self.assertIsInstance(cm.exception.__context__, KeyError)
3513+
self.assertEqual(p.apply(sqr, (3,)), 9)
3514+
3515+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
3516+
def test_apply_async_error_callback_reraises(self):
3517+
with multiprocessing.Pool(1) as p:
3518+
res = p.apply_async(raising, error_callback=reraise)
3519+
with self.assertRaises(KeyError) as cm:
3520+
res.get(support.SHORT_TIMEOUT)
3521+
# the error is not its own context
3522+
self.assertIsNone(cm.exception.__context__)
3523+
self.assertEqual(p.apply(sqr, (3,)), 9)
3524+
3525+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
3526+
def test_map_async_error_callback_reraises(self):
3527+
with multiprocessing.Pool(1) as p:
3528+
res = p.map_async(raising_map, [0], error_callback=reraise)
3529+
with self.assertRaises(KeyError) as cm:
3530+
res.get(support.SHORT_TIMEOUT)
3531+
self.assertIsNone(cm.exception.__context__)
3532+
self.assertEqual(p.apply(sqr, (3,)), 9)
3533+
3534+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
3535+
def test_apply_async_error_callback_raises_with_context(self):
3536+
# the original error is kept at the end of the context chain
3537+
with multiprocessing.Pool(1) as p:
3538+
res = p.apply_async(raising, error_callback=raise_with_context)
3539+
with self.assertRaises(CallbackError) as cm:
3540+
res.get(support.SHORT_TIMEOUT)
3541+
context = cm.exception.__context__
3542+
self.assertIsInstance(context, ZeroDivisionError)
3543+
self.assertIsInstance(context.__context__, KeyError)
3544+
self.assertEqual(p.apply(sqr, (3,)), 9)
3545+
3546+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
3547+
def test_map_async_callback_raises(self):
3548+
with multiprocessing.Pool(1) as p:
3549+
res = p.map_async(sqr, list(range(3)), callback=self._raise)
3550+
with self.assertRaises(CallbackError):
3551+
res.get(support.SHORT_TIMEOUT)
3552+
self.assertEqual(p.apply(sqr, (3,)), 9)
3553+
3554+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
3555+
def test_map_async_error_callback_raises(self):
3556+
with multiprocessing.Pool(1) as p:
3557+
res = p.map_async(raising_map, [0], error_callback=self._raise)
3558+
with self.assertRaises(CallbackError) as cm:
3559+
res.get(support.SHORT_TIMEOUT)
3560+
self.assertIsInstance(cm.exception.__context__, KeyError)
3561+
self.assertEqual(p.apply(sqr, (3,)), 9)
3562+
34613563
class _TestPoolWorkerErrors(BaseTestCase):
34623564
ALLOWED_TYPES = ('processes', )
34633565

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Fix a deadlock in :class:`multiprocessing.pool.Pool` when *callback* or
2+
*error_callback* raises an exception.
3+
It killed the thread which handles results, so that the pool hung forever.
4+
The exception is now the result of the job,
5+
as an error raised while iterating the input,
6+
and is raised by :meth:`!AsyncResult.get`.

0 commit comments

Comments
 (0)