Repository: twisted/twisted

Issue:
Remove ReactorBase._handleSignals
This is a small(ish) refactoring of how `ReactorBase` and related classes hook up their signal handling logic.  The actual signal handling logic is intended to be the same as it was before (as is all other public-facing behavior).  This has been pulled out of the branch for #11685.  The goal of #11685 is to make large pieces of reactor functionality available as independently usable pieces with good composition properties. The branch there has gotten too large to manage. This is one step towards making it small enough to be able to land. Ultimately the work in #11685 goes much further than this change but this one step makes it easier to get there (even though some of this specific factoring will soon be obsolete).

`ReactorBase._handleSignals` is called near `reactor.run` and is meant to install signal handlers for signals like SIGINT (stop the reactor).  `PosixReactorBase` overrides this to add SIGCHLD (notice child exit) handling.  `GlibReactorBase` overrides it to play nicely with glib. Also, wxreactor ... eh.

This refactoring puts all of the "application" logic (how to install the handlers and - to some extent - what to do when the signals arrive) into some new classes which implement a `SignalHandling` protocol.  `ReactorBase` is responsible for make sure one of these is created, installed, and cleaned up at the right times.  Each reactor is responsible for defining a factory that says what its correct signal handling logic is.

Presently the factory is defined as a private method of `ReactorBase` which I think is not amazing.  The other idea I can think of is to make it an `__init__` argument.  However, `PosixReactorBase` wants to compose its logic with that of `ReactorBase` which makes this a bit awkward.

Much of this logic will move around _anyway_ as more of #11685 lands so perhaps this is a fine incremental step and we'll figure out something better later?


Discussion:

Patch:
diff --git a/src/twisted/internet/_glibbase.py b/src/twisted/internet/_glibbase.py
index 8f8eb5edcf..4a6d1323ab 100644
--- a/src/twisted/internet/_glibbase.py
+++ b/src/twisted/internet/_glibbase.py
@@ -21,6 +21,7 @@ from twisted.internet import posixbase
 from twisted.internet.abstract import FileDescriptor
 from twisted.internet.interfaces import IReactorFDSet, IReadDescriptor, IWriteDescriptor
 from twisted.python import log
+from twisted.python.monkey import MonkeyPatcher
 from ._signals import _UnixWaker
 
 
@@ -56,11 +57,33 @@ class GlibWaker(_UnixWaker):
     Run scheduled events after waking up.
     """
 
+    def __init__(self, reactor):
+        super().__init__()
+        self.reactor = reactor
+
     def doRead(self) -> None:
         super().doRead()
         self.reactor._simulate()
 
 
+def _signalGlue():
+    """
+    Integrate glib's wakeup file descriptor usage and our own.
+
+    Python supports only one wakeup file descriptor at a time and both Twisted
+    and glib want to use it.
+
+    This is a context manager that can be wrapped around the whole glib
+    reactor main loop which makes our signal handling work with glib's signal
+    handling.
+    """
+    from gi import _ossighelper as signalGlue  # type: ignore[import]
+
+    patcher = MonkeyPatcher()
+    patcher.addPatch(signalGlue, "_wakeup_fd_is_active", True)
+    return patcher
+
+
 def _loopQuitter(
     idleAdd: Callable[[Callable[[], None]], None], loopQuit: Callable[[], None]
 ) -> Callable[[], None]:
@@ -106,7 +129,8 @@ class GlibReactorBase(posixbase.PosixReactorBase, posixbase._PollLikeMixin):
 
     # Install a waker that knows it needs to call C{_simulate} in order to run
     # callbacks queued from a thread:
-    _wakerFactory = GlibWaker
+    def _wakerFactory(self) -> GlibWaker:
+        return GlibWaker(self)
 
     def __init__(self, glib_module: Any, gtk_module: Any, useGtk: bool = False) -> None:
         self._simtag = None
@@ -141,14 +165,29 @@ class GlibReactorBase(posixbase.PosixReactorBase, posixbase._PollLikeMixin):
         self._crash = _loopQuitter(self._glib.idle_add, self.loop.quit)
         self._run = self.loop.run
 
-    def _handleSignals(self):
+    def _reallyStartRunning(self):
+        """
+        Make sure the reactor's signal handlers are installed despite any
+        outside interference.
+        """
         # First, install SIGINT and friends:
-        super()._handleSignals()
+        super()._reallyStartRunning()
+
         # Next, since certain versions of gtk will clobber our signal handler,
         # set all signal handlers again after the event loop has started to
-        # ensure they're *really* set. We don't call this twice so we don't
-        # leak file descriptors created in the SIGCHLD initialization:
-        self.callLater(0, posixbase.PosixReactorBase._handleSignals, self)
+        # ensure they're *really* set.
+        #
+        # We don't actually know which versions of gtk do this so this might
+        # be obsolete.  If so, that would be great and this whole method can
+        # go away.  Someone needs to find out, though.
+        #
+        # https://github.com/twisted/twisted/issues/11762
+
+        def reinitSignals():
+            self._signals.uninstall()
+            self._signals.install()
+
+        self.callLater(0, reinitSignals)
 
     # The input_add function in pygtk1 checks for objects with a
     # 'fileno' method and, if present, uses the result of that method
@@ -170,7 +209,10 @@ class GlibReactorBase(posixbase.PosixReactorBase, posixbase._PollLikeMixin):
             fileno = source
             wrapper = callback
         return self._glib.io_add_watch(
-            fileno, condition, wrapper, priority=self._glib.PRIORITY_DEFAULT_IDLE
+            fileno,
+            self._glib.PRIORITY_DEFAULT_IDLE,
+            condition,
+            wrapper,
         )
 
     def _ioEventCallback(self, source, condition):
@@ -288,10 +330,11 @@ class GlibReactorBase(posixbase.PosixReactorBase, posixbase._PollLikeMixin):
         """
         Run the reactor.
         """
-        self.callWhenRunning(self._reschedule)
-        self.startRunning(installSignalHandlers=installSignalHandlers)
-        if self._started:
-            self._run()
+        with _signalGlue():
+            self.callWhenRunning(self._reschedule)
+            self.startRunning(installSignalHandlers=installSignalHandlers)
+            if self._started:
+                self._run()
 
     def callLater(self, *args, **kwargs):
         """
diff --git a/src/twisted/internet/_signals.py b/src/twisted/internet/_signals.py
index 6c151decf6..fa878f6cba 100644
--- a/src/twisted/internet/_signals.py
+++ b/src/twisted/internet/_signals.py
@@ -18,13 +18,12 @@ the main thread (waiting on select() or another event notification
 API) may not wake up leading to an arbitrary delay before the child
 termination is noticed.
 
-The basic solution to all these issues involves enabling SA_RESTART
-(ie, disabling system call interruption) and registering a C signal
-handler which writes a byte to a pipe.  The other end of the pipe is
-registered with the event loop, allowing it to wake up shortly after
-SIGCHLD is received.  See L{twisted.internet.posixbase._SIGCHLDWaker}
-for the implementation of the event loop side of this solution.  The
-use of a pipe this way is known as the U{self-pipe
+The basic solution to all these issues involves enabling SA_RESTART (ie,
+disabling system call interruption) and registering a C signal handler which
+writes a byte to a pipe.  The other end of the pipe is registered with the
+event loop, allowing it to wake up shortly after SIGCHLD is received.  See
+L{_SIGCHLDWaker} for the implementation of the event loop side of this
+solution.  The use of a pipe this way is known as the U{self-pipe
 trick<http://cr.yp.to/docs/selfpipe.html>}.
 
 From Python version 2.6, C{signal.siginterrupt} and C{signal.set_wakeup_fd}
@@ -32,23 +31,32 @@ provide the necessary C signal handler which writes to the pipe to be
 registered with C{SA_RESTART}.
 """
 
+from __future__ import annotations
 
 import contextlib
 import errno
 import os
 import signal
 import socket
+from types import FrameType
+from typing import Callable, Optional, Sequence
 
 from zope.interface import Attribute, Interface, implementer
 
+from attrs import define, frozen
+from typing_extensions import Protocol, TypeAlias
+
+from twisted.internet.interfaces import IReadDescriptor
 from twisted.python import failure, log, util
 from twisted.python.runtime import platformType
 
 if platformType == "posix":
     from . import fdesc, process
 
+SignalHandler: TypeAlias = Callable[[int, Optional[FrameType]], None]
+
 
-def installHandler(fd):
+def installHandler(fd: int) -> int:
     """
     Install a signal handler which will write a byte to C{fd} when
     I{SIGCHLD} is received.
@@ -59,7 +67,8 @@ def installHandler(fd):
 
     @param fd: The file descriptor to which to write when I{SIGCHLD} is
         received.
-    @type fd: C{int}
+
+    @return: The file descriptor previously configured for this use.
     """
     if fd == -1:
         signal.signal(signal.SIGCHLD, signal.SIG_DFL)
@@ -80,6 +89,175 @@ def isDefaultHandler():
     return signal.getsignal(signal.SIGCHLD) == signal.SIG_DFL
 
 
+class SignalHandling(Protocol):
+    """
+    The L{SignalHandling} protocol enables customizable signal-handling
+    behaviors for reactors.
+
+    A value that conforms to L{SignalHandling} has install and uninstall hooks
+    that are called by a reactor at the correct times to have the (typically)
+    process-global effects necessary for dealing with signals.
+    """
+
+    def install(self) -> None:
+        """
+        Install the signal handlers.
+        """
+
+    def uninstall(self) -> None:
+        """
+        Restore signal handlers to their original state.
+        """
+
+
+@frozen
+class _WithoutSignalHandling:
+    """
+    A L{SignalHandling} implementation that does no signal handling.
+
+    This is the implementation of C{installSignalHandlers=False}.
+    """
+
+    def install(self) -> None:
+        """
+        Do not install any signal handlers.
+        """
+
+    def uninstall(self) -> None:
+        """
+        Do nothing because L{install} installed nothing.
+        """
+
+
+@frozen
+class _WithSignalHandling:
+    """
+    A reactor core helper that can manage signals: it installs signal handlers
+    at start time.
+    """
+
+    _sigInt: SignalHandler
+    _sigBreak: SignalHandler
+    _sigTerm: SignalHandler
+
+    def install(self) -> None:
+        """
+        Install the signal handlers for the Twisted event loop.
+        """
+        if signal.getsignal(signal.SIGINT) == signal.default_int_handler:
+            # only handle if there isn't already a handler, e.g. for Pdb.
+            signal.signal(signal.SIGINT, self._sigInt)
+        signal.signal(signal.SIGTERM, self._sigTerm)
+
+        # Catch Ctrl-Break in windows
+        SIGBREAK = getattr(signal, "SIGBREAK", None)
+        if SIGBREAK is not None:
+            signal.signal(SIGBREAK, self._sigBreak)
+
+    def uninstall(self) -> None:
+        """
+        At the moment, do nothing (for historical reasons).
+        """
+        # This should really do something.
+        # https://github.com/twisted/twisted/issues/11761
+
+
+@define
+class _MultiSignalHandling:
+    """
+    An implementation of L{SignalHandling} which propagates protocol
+    method calls to a number of other implementations.
+
+    This supports composition of multiple signal handling implementations into
+    a single object so the reactor doesn't have to be concerned with how those
+    implementations are factored.
+
+    @ivar _signalHandlings: The other C{SignalHandling} implementations to
+        which to propagate calls.
+
+    @ivar _installed: If L{install} has been called but L{uninstall} has not.
+        This is used to avoid double cleanup which otherwise results (at least
+        during test suite runs) because twisted.internet.reactormixins doesn't
+        keep track of whether a reactor has run or not but always invokes its
+        cleanup logic.
+    """
+
+    _signalHandlings: Sequence[SignalHandling]
+    _installed: bool = False
+
+    def install(self) -> None:
+        for d in self._signalHandlings:
+            d.install()
+        self._installed = True
+
+    def uninstall(self) -> None:
+        if self._installed:
+            for d in self._signalHandlings:
+                d.uninstall()
+            self._installed = False
+
+
+@define
+class _ChildSignalHandling:
+    """
+    Signal handling behavior which supports I{SIGCHLD} for notification about
+    changes to child process state.
+
+    @ivar _childWaker: L{None} or a reference to the L{_SIGCHLDWaker} which is
+        used to properly notice child process termination.  This is L{None}
+        when this handling behavior is not installed and non-C{None}
+        otherwise.  This is mostly an unfortunate implementation detail due to
+        L{_SIGCHLDWaker} allocating file descriptors as a side-effect of its
+        initializer.
+    """
+
+    _addInternalReader: Callable[[IReadDescriptor], object]
+    _removeInternalReader: Callable[[IReadDescriptor], object]
+    _childWaker: Optional[_SIGCHLDWaker] = None
+
+    def install(self) -> None:
+        """
+        Extend the basic signal handling logic to also support handling
+        SIGCHLD to know when to try to reap child processes.
+        """
+        # This conditional should probably not be necessary.
+        # https://github.com/twisted/twisted/issues/11763
+        if self._childWaker is None:
+            self._childWaker = _SIGCHLDWaker()
+            self._addInternalReader(self._childWaker)
+        self._childWaker.install()
+
+        # Also reap all processes right now, in case we missed any
+        # signals before we installed the SIGCHLD waker/handler.
+        # This should only happen if someone used spawnProcess
+        # before calling reactor.run (and the process also exited
+        # already).
+        process.reapAllProcesses()
+
+    def uninstall(self) -> None:
+        """
+        If a child waker was created and installed, uninstall it now.
+
+        Since this disables reactor functionality and is only called when the
+        reactor is stopping, it doesn't provide any directly useful
+        functionality, but the cleanup of reactor-related process-global state
+        that it does helps in unit tests involving multiple reactors and is
+        generally just a nice thing.
+        """
+        assert self._childWaker is not None
+
+        # XXX This would probably be an alright place to put all of the
+        # cleanup code for all internal readers (here and in the base class,
+        # anyway).  See #3063 for that cleanup task.
+        self._removeInternalReader(self._childWaker)
+        self._childWaker.uninstall()
+        self._childWaker.connectionLost(failure.Failure(Exception("uninstalled")))
+
+        # We just spoiled the current _childWaker so throw it away.  We can
+        # make a new one later if need be.
+        self._childWaker = None
+
+
 class _IWaker(Interface):
     """
     Interface to wake up the event loop based on the self-pipe trick.
@@ -120,9 +298,8 @@ class _SocketWaker(log.Logger):
 
     disconnected = 0
 
-    def __init__(self, reactor):
+    def __init__(self) -> None:
         """Initialize."""
-        self.reactor = reactor
         # Following select_trigger (from asyncore)'s example;
         client = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         client.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
@@ -133,8 +310,8 @@ class _SocketWaker(log.Logger):
             server.listen(1)
             client.connect(server.getsockname())
             reader, clientaddr = server.accept()
-        client.setblocking(0)
-        reader.setblocking(0)
+        client.setblocking(False)
+        reader.setblocking(False)
         self.r = reader
         self.w = client
         self.fileno = self.r.fileno
@@ -161,6 +338,7 @@ class _SocketWaker(log.Logger):
         self.w.close()
 
 
+@implementer(IReadDescriptor)
 class _FDWaker(log.Logger):
     """
     The I{self-pipe trick<http://cr.yp.to/docs/selfpipe.html>}, used to wake
@@ -178,12 +356,11 @@ class _FDWaker(log.Logger):
 
     disconnected = 0
 
-    i = None
-    o = None
+    i: int
+    o: int
 
-    def __init__(self, reactor):
+    def __init__(self) -> None:
         """Initialize."""
-        self.reactor = reactor
         self.i, self.o = os.pipe()
         fdesc.setNonBlocking(self.i)
         fdesc._setCloseOnExec(self.i)
@@ -191,7 +368,7 @@ class _FDWaker(log.Logger):
         fdesc._setCloseOnExec(self.o)
         self.fileno = lambda: self.i
 
-    def doRead(self):
+    def doRead(self) -> None:
         """
         Read some bytes from the pipe and discard them.
         """
@@ -240,26 +417,22 @@ else:
 
 class _SIGCHLDWaker(_FDWaker):
     """
-    L{_SIGCHLDWaker} can wake up a reactor whenever C{SIGCHLD} is
-    received.
+    L{_SIGCHLDWaker} can wake up a reactor whenever C{SIGCHLD} is received.
     """
 
-    def __init__(self, reactor):
-        _FDWaker.__init__(self, reactor)
-
-    def install(self):
+    def install(self) -> None:
         """
         Install the handler necessary to make this waker active.
         """
         installHandler(self.o)
 
-    def uninstall(self):
+    def uninstall(self) -> None:
         """
         Remove the handler which makes this waker active.
         """
         installHandler(-1)
 
-    def doRead(self):
+    def doRead(self) -> None:
         """
         Having woken up the reactor in response to receipt of
         C{SIGCHLD}, reap the process which exited.
@@ -268,5 +441,5 @@ class _SIGCHLDWaker(_FDWaker):
         writeable, which happens soon after any call to the C{wakeUp}
         method.
         """
-        _FDWaker.doRead(self)
+        super().doRead()
         process.reapAllProcesses()
diff --git a/src/twisted/internet/base.py b/src/twisted/internet/base.py
index b7161392ff..33c2f38e47 100644
--- a/src/twisted/internet/base.py
+++ b/src/twisted/internet/base.py
@@ -58,6 +58,7 @@ from twisted.internet.protocol import ClientFactory
 from twisted.python import log, reflect
 from twisted.python.failure import Failure
 from twisted.python.runtime import platform, seconds as runtimeSeconds
+from ._signals import SignalHandling, _WithoutSignalHandling, _WithSignalHandling
 
 if TYPE_CHECKING:
     from twisted.internet.tcp import Client
@@ -591,6 +592,13 @@ class ReactorBase(PluggableResolverMixin):
         register the thread it is running in as the I/O thread when it starts.
         If C{True}, registration will be done, otherwise it will not be.
     @ivar _exitSignal: See L{_ISupportsExitSignalCapturing._exitSignal}
+
+    @ivar _installSignalHandlers: A flag which indicates whether any signal
+        handlers will be installed during startup.  This includes handlers for
+        SIGCHLD to monitor child processes, and SIGINT, SIGTERM, and SIGBREAK
+
+    @ivar _signals: An object which knows how to install and uninstall the
+        reactor's signal-handling behavior.
     """
 
     _registerAsIOThread = True
@@ -600,6 +608,12 @@ class ReactorBase(PluggableResolverMixin):
     usingThreads = False
     _exitSignal = None
 
+    # Set to something meaningful between startRunning and shortly before run
+    # returns.  We don't know the value to be used by `run` until that method
+    # itself is called and we learn the value of installSignalHandlers.
+    # However, we can use a no-op implementation until then.
+    _signals: SignalHandling = _WithoutSignalHandling()
+
     __name__ = "twisted.internet.reactor"
 
     def __init__(self) -> None:
@@ -630,34 +644,55 @@ class ReactorBase(PluggableResolverMixin):
         self.installWaker()
 
     # Signal handling pieces
-    _installSignalHandlers = False
+    _installSignalHandlers: bool = False
 
-    def _handleSignals(self) -> None:
+    def _makeSignalHandling(self, installSignalHandlers: bool) -> SignalHandling:
         """
-        Install the signal handlers for the Twisted event loop.
+        Get an appropriate signal handling object.
+
+        @param installSignalHandlers: Indicate whether to even try to do any
+            signal handling.  If C{False} then the result will be a no-op
+            implementation.
         """
-        try:
-            import signal
-        except ImportError:
-            log.msg(
-                "Warning: signal module unavailable -- "
-                "not installing signal handlers."
-            )
-            return
+        if installSignalHandlers:
+            return self._signalsFactory()
+        return _WithoutSignalHandling()
 
-        if signal.getsignal(signal.SIGINT) == signal.default_int_handler:
-            # only handle if there isn't already a handler, e.g. for Pdb.
-            signal.signal(signal.SIGINT, self.sigInt)
-        signal.signal(signal.SIGTERM, self.sigTerm)
+    def _signalsFactory(self) -> SignalHandling:
+        """
+        Get a signal handling object that implements the basic behavior of
+        stopping the reactor on SIGINT, SIGBREAK, and SIGTERM.
+        """
+        return _WithSignalHandling(
+            self.sigInt,
+            self.sigBreak,
+            self.sigTerm,
+        )
 
-        # Catch Ctrl-Break in windows
-        SIGBREAK = getattr(signal, "SIGBREAK", None)
-        if SIGBREAK is not None:
-            signal.signal(SIGBREAK, self.sigBreak)
+    def _addInternalReader(self, reader: IReadDescriptor) -> None:
+        """
+        Add a read descriptor which is part of the implementation of the
+        reactor itself.
+
+        The read descriptor will not be removed by L{IReactorFDSet.removeAll}.
+        """
+        self._internalReaders.add(reader)
+        self.addReader(reader)
+
+    def _removeInternalReader(self, reader: IReadDescriptor) -> None:
+        """
+        Remove a read descriptor which is part of the implementation of the
+        reactor itself.
+        """
+        self._internalReaders.remove(reader)
+        self.removeReader(reader)
 
     def run(self, installSignalHandlers: bool = True) -> None:
         self.startRunning(installSignalHandlers=installSignalHandlers)
-        self.mainLoop()
+        try:
+            self.mainLoop()
+        finally:
+            self._signals.uninstall()
 
     def mainLoop(self) -> None:
         while self._started:
@@ -887,11 +922,15 @@ class ReactorBase(PluggableResolverMixin):
             handlers for a number of (implementation-defined) signals should be
             installed during startup.
         """
-        self._installSignalHandlers = installSignalHandlers
         if self._started:
             raise error.ReactorAlreadyRunning()
         if self._startedBefore:
             raise error.ReactorNotRestartable()
+
+        self._signals.uninstall()
+        self._installSignalHandlers = installSignalHandlers
+        self._signals = self._makeSignalHandling(installSignalHandlers)
+
         self._started = True
         self._stopped = False
         if self._registerAsIOThread:
@@ -911,7 +950,7 @@ class ReactorBase(PluggableResolverMixin):
             # (perhaps some before-startup triggers don't want there to be a
             # custom SIGCHLD handler so that they can run child processes with
             # some blocking api).
-            self._handleSignals()
+            self._signals.install()
 
     # IReactorTime
 
@@ -1116,7 +1155,7 @@ class ReactorBase(PluggableResolverMixin):
             )
 
         def _uninstallHandler(self) -> None:
-            pass
+            self._signals.uninstall()
 
         def _stopThreadPool(self) -> None:
             """
diff --git a/src/twisted/internet/cfreactor.py b/src/twisted/internet/cfreactor.py
index ebe09fea69..333ab49760 100644
--- a/src/twisted/internet/cfreactor.py
+++ b/src/twisted/internet/cfreactor.py
@@ -74,6 +74,10 @@ class _WakerPlus(_UnixWaker):
     do.
     """
 
+    def __init__(self, reactor):
+        super().__init__()
+        self.reactor = reactor
+
     def doRead(self):
         """
         Wake up the loop and force C{runUntilCurrent} to run immediately in the
@@ -136,15 +140,8 @@ class CFReactor(PosixReactorBase):
         self._cfrunloop = runLoop
         PosixReactorBase.__init__(self)
 
-    def installWaker(self):
-        """
-        Override C{installWaker} in order to use L{_WakerPlus}; otherwise this
-        should be exactly the same as the parent implementation.
-        """
-        if not self.waker:
-            self.waker = _WakerPlus(self)
-            self._internalReaders.add(self.waker)
-            self.addReader(self.waker)
+    def _wakerFactory(self) -> _WakerPlus:
+        return _WakerPlus(self)
 
     def _socketCallback(
         self, cfSocket, callbackType, ignoredAddress, ignoredData, context
diff --git a/src/twisted/internet/posixbase.py b/src/twisted/internet/posixbase.py
index d3c37df832..bd160ec865 100644
--- a/src/twisted/internet/posixbase.py
+++ b/src/twisted/internet/posixbase.py
@@ -30,7 +30,13 @@ from twisted.internet.interfaces import (
 from twisted.internet.main import CONNECTION_DONE, CONNECTION_LOST
 from twisted.python import failure, log
 from twisted.python.runtime import platform, platformType
-from ._signals import _SIGCHLDWaker, _Waker
+from ._signals import (
+    SignalHandling,
+    _ChildSignalHandling,
+    _IWaker,
+    _MultiSignalHandling,
+    _Waker,
+)
 
 # Exceptions that doSelect might return frequently
 _NO_FILENO = error.ConnectionFdescWentAway("Handler has no fileno method")
@@ -116,9 +122,12 @@ class PosixReactorBase(_DisconnectSelectableMixin, ReactorBase):
         which is used to properly notice child process termination.
     """
 
+    _childWaker = None
+
     # Callable that creates a waker, overrideable so that subclasses can
     # substitute their own implementation:
-    _wakerFactory = _Waker
+    def _wakerFactory(self) -> _IWaker:
+        return _Waker()
 
     def installWaker(self):
         """
@@ -128,47 +137,33 @@ class PosixReactorBase(_DisconnectSelectableMixin, ReactorBase):
         the reactor. On Windows we use a pair of sockets.
         """
         if not self.waker:
-            self.waker = self._wakerFactory(self)
+            self.waker = self._wakerFactory()
             self._internalReaders.add(self.waker)
             self.addReader(self.waker)
 
-    _childWaker = None
+    def _signalsFactory(self) -> SignalHandling:
+        """
+        Customize reactor signal handling to support child processes on POSIX
+        platforms.
+        """
+        baseHandling = super()._signalsFactory()
+        # If we're on a platform that uses signals for process event signaling
+        if platformType == "posix":
+            # Compose ...
+            return _MultiSignalHandling(
+                (
+                    # the base signal handling behavior ...
+                    baseHandling,
+                    # with our extra SIGCHLD handling behavior.
+                    _ChildSignalHandling(
+                        self._addInternalReader,
+                        self._removeInternalReader,
+                    ),
+                )
+            )
 
-    def _handleSignals(self):
-        """
-        Extend the basic signal handling logic to also support
-        handling SIGCHLD to know when to try to reap child processes.
-        """
-        super()._handleSignals()
-        if platformType == "posix" and processEnabled:
-            if not self._childWaker:
-                self._childWaker = _SIGCHLDWaker(self)
-                self._internalReaders.add(self._childWaker)
-                self.addReader(self._childWaker)
-            self._childWaker.install()
-            # Also reap all processes right now, in case we missed any
-            # signals before we installed the SIGCHLD waker/handler.
-            # This should only happen if someone used spawnProcess
-            # before calling reactor.run (and the process also exited
-            # already).
-            process.reapAllProcesses()
-
-    def _uninstallHandler(self):
-        """
-        If a child waker was created and installed, uninstall it now.
-
-        Since this disables reactor functionality and is only called
-        when the reactor is stopping, it doesn't provide any directly
-        useful functionality, but the cleanup of reactor-related
-        process-global state that it does helps in unit tests
-        involving multiple reactors and is generally just a nice
-        thing.
-        """
-        # XXX This would probably be an alright place to put all of
-        # the cleanup code for all internal readers (here and in the
-        # base class, anyway).  See #3063 for that cleanup task.
-        if self._childWaker:
-            self._childWaker.uninstall()
+        # Otherwise just use the base behavior
+        return baseHandling
 
     # IReactorProcess
 
diff --git a/src/twisted/internet/wxreactor.py b/src/twisted/internet/wxreactor.py
index 67e96bf2ca..b988724dfa 100644
--- a/src/twisted/internet/wxreactor.py
+++ b/src/twisted/internet/wxreactor.py
@@ -84,7 +84,7 @@ class WxReactor(_threadedselect.ThreadedSelectReactor):
             signal.signal(signal.SIGINT, signal.default_int_handler)
         except ImportError:
             return
-        self._handleSignals()
+        self._signals.install()
 
     def stop(self):
         """


Test patch:
diff --git a/src/twisted/internet/test/test_posixbase.py b/src/twisted/internet/test/test_posixbase.py
index 605a02be1d..7dd58a92bc 100644
--- a/src/twisted/internet/test/test_posixbase.py
+++ b/src/twisted/internet/test/test_posixbase.py
@@ -7,6 +7,7 @@ Tests for L{twisted.internet.posixbase} and supporting code.
 import os
 
 from twisted.internet.defer import Deferred
+from twisted.internet.interfaces import IReadDescriptor
 from twisted.internet.posixbase import PosixReactorBase, _Waker
 from twisted.internet.protocol import ServerFactory
 from twisted.python.runtime import platform
@@ -96,7 +97,7 @@ class PosixReactorBaseTests(WarningCheckerTestCase):
 
     def test_removeAllSkipsInternalReaders(self):
         """
-        Any L{IReadDescriptors} in L{PosixReactorBase._internalReaders} are
+        Any L{IReadDescriptor}s in L{PosixReactorBase._internalReaders} are
         left alone by L{PosixReactorBase._removeAll}.
         """
         reactor = TrivialReactor()
@@ -155,12 +156,17 @@ class TimeoutReportReactor(PosixReactorBase):
         self.iterationTimeout = Deferred()
         self.now = 100
 
-    def addReader(self, reader):
+    def addReader(self, reader: IReadDescriptor) -> None:
         """
         Ignore the reader.  This is necessary because the waker will be
         added.  However, we won't actually monitor it for any events.
         """
 
+    def removeReader(self, reader: IReadDescriptor) -> None:
+        """
+        See L{addReader}.
+        """
+
     def removeAll(self):
         """
         There are no readers or writers, so there is nothing to remove.
@@ -331,7 +337,7 @@ class WakerTests(WarningCheckerTestCase):
         """
         No warnings are generated when constructing the waker.
         """
-        waker = _Waker(reactor=None)
+        waker = _Waker()
 
         warnings = self.flushWarnings()
         self.assertEqual(len(warnings), 0, warnings)
diff --git a/src/twisted/internet/test/test_sigchld.py b/src/twisted/internet/test/test_sigchld.py
index 73cf87956b..6385850012 100644
--- a/src/twisted/internet/test/test_sigchld.py
+++ b/src/twisted/internet/test/test_sigchld.py
@@ -11,7 +11,6 @@ import errno
 import os
 import signal
 
-from twisted.python.log import msg
 from twisted.python.runtime import platformType
 from twisted.trial.unittest import SynchronousTestCase
 
@@ -45,34 +44,16 @@ class SetWakeupSIGCHLDTests(SynchronousTestCase):
         Save the current SIGCHLD handler as reported by L{signal.signal} and
         the current file descriptor registered with L{installHandler}.
         """
-        handler = signal.getsignal(signal.SIGCHLD)
-        if handler != signal.SIG_DFL:
-            self.signalModuleHandler = handler
-            signal.signal(signal.SIGCHLD, signal.SIG_DFL)
-        else:
-            self.signalModuleHandler = None
-
+        self.signalModuleHandler = signal.getsignal(signal.SIGCHLD)
         self.oldFD = installHandler(-1)
 
-        if self.signalModuleHandler is not None and self.oldFD != -1:
-            msg(
-                "Previous test didn't clean up after its SIGCHLD setup: %r %r"
-                % (self.signalModuleHandler, self.oldFD)
-            )
-
     def tearDown(self):
         """
         Restore whatever signal handler was present when setUp ran.
         """
         # If tests set up any kind of handlers, clear them out.
-        installHandler(-1)
-        signal.signal(signal.SIGCHLD, signal.SIG_DFL)
-
-        # Now restore whatever the setup was before the test ran.
-        if self.signalModuleHandler is not None:
-            signal.signal(signal.SIGCHLD, self.signalModuleHandler)
-        elif self.oldFD != -1:
-            installHandler(self.oldFD)
+        installHandler(self.oldFD)
+        signal.signal(signal.SIGCHLD, self.signalModuleHandler)
 
     def test_isDefaultHandler(self):
         """