Skip to content

Commit

Permalink
arbiter: Remove SIG_QUEUE and only use PIPE instead
Browse files Browse the repository at this point in the history
Since memory allocations aren't recommended when we're in signal
handling context, it's most likely unsafe to manipulate a python list
inside such context.

Instead, when we're in a signal handler, we write the signal number to
a pipe, and then we read the pipe once we've exited the signal
context. This way, we won't to any memory allocation in a signal
context, as the handling takes place in the "normal" context.

So after this commit, PIPE is used both for waking up the arbiter
process (prematurely before the timeout has been reached), and for
sending what incoming signal we've received.
  • Loading branch information
sylt committed Feb 2, 2024
1 parent b6d8524 commit 21b6271
Showing 1 changed file with 22 additions and 21 deletions.
43 changes: 22 additions & 21 deletions gunicorn/arbiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ class Arbiter(object):
WORKERS = {}
PIPE = []

# Indicator used to signal that the arbiter main thread has work to do
WAKE_UP = 0

# I love dynamic languages
SIG_QUEUE = []
SIGNALS = [getattr(signal, "SIG%s" % x)
for x in "CHLD CLD HUP QUIT INT TERM TTIN TTOU USR1 USR2 WINCH".split()]
SIG_NAMES = dict(
Expand Down Expand Up @@ -187,10 +189,16 @@ def init_signals(self):
for s in self.SIGNALS:
signal.signal(s, self.signal)

def write_message_to_pipe(self, one_byte_message):
try:
os.write(self.PIPE[1], one_byte_message.to_bytes(1))
except IOError as e:
if e.errno not in [errno.EAGAIN, errno.EINTR]:
raise

def signal(self, sig, frame):
if len(self.SIG_QUEUE) < 5:
self.SIG_QUEUE.append(sig)
self.wakeup()
""" Note: Signal context! No logging or memory allocations allowed. """
self.write_message_to_pipe(sig)

def run(self):
"Main master loop."
Expand All @@ -203,13 +211,13 @@ def run(self):
while True:
self.maybe_promote_master()

sig = self.SIG_QUEUE.pop(0) if self.SIG_QUEUE else None
if sig is None:
self.sleep()
event_or_signal = self.wait_for_event(timeout_s=1.0)
if event_or_signal is None or event_or_signal == self.WAKE_UP:
self.murder_workers()
self.manage_workers()
continue

sig = event_or_signal
if sig not in self.SIG_NAMES:
self.log.info("Ignoring unknown signal: %s", sig)
continue
Expand Down Expand Up @@ -332,11 +340,7 @@ def wakeup(self):
"""\
Wake up the arbiter by writing to the PIPE
"""
try:
os.write(self.PIPE[1], b'.')
except IOError as e:
if e.errno not in [errno.EAGAIN, errno.EINTR]:
raise
self.write_message_to_pipe(self.WAKE_UP)

def halt(self, reason=None, exit_status=0):
""" halt arbiter """
Expand All @@ -352,17 +356,14 @@ def halt(self, reason=None, exit_status=0):
self.cfg.on_exit(self)
sys.exit(exit_status)

def sleep(self):
"""\
Sleep until PIPE is readable or we timeout.
A readable PIPE means a signal occurred.
"""
def wait_for_event(self, timeout_s):
""" Waits until PIPE is readable or we timeout. """
try:
ready = select.select([self.PIPE[0]], [], [], 1.0)
ready = select.select([self.PIPE[0]], [], [], timeout_s)
if not ready[0]:
return
while os.read(self.PIPE[0], 1):
pass
return None

return int.from_bytes(os.read(self.PIPE[0], 1))
except (select.error, OSError) as e:
# TODO: select.error is a subclass of OSError since Python 3.3.
error_number = getattr(e, 'errno', e.args[0])
Expand Down

0 comments on commit 21b6271

Please sign in to comment.