Skip to content

Commit

Permalink
gh-127586: multiprocessing.Pool does not properly restore blocked sig…
Browse files Browse the repository at this point in the history
…nals (try 2) (GH-128011)

Correct pthread_sigmask in resource_tracker to restore old signals

Using SIG_UNBLOCK to remove blocked "ignored signals" may accidentally
cause side effects if the calling parent already had said signals
blocked to begin with and did not intend to unblock them when
creating a pool. Use SIG_SETMASK instead with the previous mask of
blocked signals to restore the original blocked set.

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
  • Loading branch information
3 people authored Dec 27, 2024
1 parent 64173cd commit aeb9b65
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
7 changes: 4 additions & 3 deletions Lib/multiprocessing/resource_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,14 @@ def ensure_running(self):
# that can make the child die before it registers signal handlers
# for SIGINT and SIGTERM. The mask is unregistered after spawning
# the child.
prev_sigmask = None
try:
if _HAVE_SIGMASK:
signal.pthread_sigmask(signal.SIG_BLOCK, _IGNORED_SIGNALS)
prev_sigmask = signal.pthread_sigmask(signal.SIG_BLOCK, _IGNORED_SIGNALS)
pid = util.spawnv_passfds(exe, args, fds_to_pass)
finally:
if _HAVE_SIGMASK:
signal.pthread_sigmask(signal.SIG_UNBLOCK, _IGNORED_SIGNALS)
if prev_sigmask is not None:
signal.pthread_sigmask(signal.SIG_SETMASK, prev_sigmask)
except:
os.close(w)
raise
Expand Down
21 changes: 21 additions & 0 deletions Lib/test/_test_multiprocessing.py
Original file line number Diff line number Diff line change
Expand Up @@ -6045,6 +6045,27 @@ def test_resource_tracker_exit_code(self):
cleanup=cleanup,
)

@unittest.skipUnless(hasattr(signal, "pthread_sigmask"), "pthread_sigmask is not available")
def test_resource_tracker_blocked_signals(self):
#
# gh-127586: Check that resource_tracker does not override blocked signals of caller.
#
from multiprocessing.resource_tracker import ResourceTracker
orig_sigmask = signal.pthread_sigmask(signal.SIG_BLOCK, set())
signals = {signal.SIGTERM, signal.SIGINT, signal.SIGUSR1}

try:
for sig in signals:
signal.pthread_sigmask(signal.SIG_SETMASK, {sig})
self.assertEqual(signal.pthread_sigmask(signal.SIG_BLOCK, set()), {sig})
tracker = ResourceTracker()
tracker.ensure_running()
self.assertEqual(signal.pthread_sigmask(signal.SIG_BLOCK, set()), {sig})
tracker._stop()
finally:
# restore sigmask to what it was before executing test
signal.pthread_sigmask(signal.SIG_SETMASK, orig_sigmask)

class TestSimpleQueue(unittest.TestCase):

@classmethod
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:class:`multiprocessing.pool.Pool` now properly restores blocked signal handlers
of the parent thread when creating processes via either *spawn* or
*forkserver*.

0 comments on commit aeb9b65

Please sign in to comment.