From 44994ec59e9543b9e3bd642d5b7c484e94bd3614 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 23 Oct 2025 18:51:07 -0500 Subject: [PATCH] Be mindful of other `SIGHUP` handlers --- synapse/app/_base.py | 100 ++++++++++++++++++++++++++----------------- 1 file changed, 60 insertions(+), 40 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index e30151dfb4..d89c52957d 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -29,6 +29,7 @@ import traceback import warnings from textwrap import indent from threading import Thread +from types import FrameType from typing import ( TYPE_CHECKING, Any, @@ -39,6 +40,7 @@ from typing import ( NoReturn, Optional, Tuple, + Union, cast, ) from wsgiref.simple_server import WSGIServer @@ -75,7 +77,6 @@ from synapse.handlers.auth import load_legacy_password_auth_providers from synapse.http.site import SynapseSite from synapse.logging.context import LoggingContext, PreserveLoggingContext from synapse.metrics import install_gc_manager, register_threadpool -from synapse.metrics.background_process_metrics import run_as_background_process from synapse.metrics.jemalloc import setup_jemalloc_stats from synapse.module_api.callbacks.spamchecker_callbacks import load_legacy_spam_checkers from synapse.module_api.callbacks.third_party_event_rules_callbacks import ( @@ -543,6 +544,61 @@ def refresh_certificate(hs: "HomeServer") -> None: logger.info("Context factories updated.") +_already_setup_sighup_handling = False +""" +Marks whether we've already successfully ran `setup_sighup_handling()`. +""" + + +def setup_sighup_handling() -> None: + """ + Set up SIGHUP handling to call registered callbacks. + + This can be called multiple times safely. + """ + global _already_setup_sighup_handling + # We only need to set things up once per process. + if _already_setup_sighup_handling: + return + + previous_sighup_handler: Union[ + Callable[[int, Optional[FrameType]], Any], int, None + ] = None + + # Set up the SIGHUP machinery. + if hasattr(signal, "SIGHUP"): + + def handle_sighup(*args: Any, **kwargs: Any) -> None: + # Tell systemd our state, if we're using it. This will silently fail if + # we're not using systemd. + sdnotify(b"RELOADING=1") + + if callable(previous_sighup_handler): + previous_sighup_handler(*args, **kwargs) + + for sighup_callbacks in _instance_id_to_sighup_callbacks_map.values(): + for func, args, kwargs in sighup_callbacks: + func(*args, **kwargs) + + sdnotify(b"READY=1") + + # We defer running the sighup handlers until next reactor tick. This + # is so that we're in a sane state, e.g. flushing the logs may fail + # if the sighup happens in the middle of writing a log entry. + def run_sighup(*args: Any, **kwargs: Any) -> None: + # `callFromThread` should be "signal safe" as well as thread + # safe. + reactor.callFromThread(handle_sighup, *args, **kwargs) + + # Register for the SIGHUP signal, chaining any existing handler as there can + # only be one handler per signal and we don't want to clobber any existing + # handlers (like the `multi_synapse` shard process in the context of Synapse Pro + # for small hosts) + previous_sighup_handler = signal.signal(signal.SIGHUP, run_sighup) + + _already_setup_sighup_handling = True + + async def start(hs: "HomeServer", freeze: bool = True) -> None: """ Start a Synapse server or worker. @@ -582,45 +638,9 @@ async def start(hs: "HomeServer", freeze: bool = True) -> None: name="gai_resolver", server_name=server_name, threadpool=resolver_threadpool ) - # Set up the SIGHUP machinery. - if hasattr(signal, "SIGHUP"): - - def handle_sighup(*args: Any, **kwargs: Any) -> "defer.Deferred[None]": - async def _handle_sighup(*args: Any, **kwargs: Any) -> None: - # Tell systemd our state, if we're using it. This will silently fail if - # we're not using systemd. - sdnotify(b"RELOADING=1") - - for sighup_callbacks in _instance_id_to_sighup_callbacks_map.values(): - for func, args, kwargs in sighup_callbacks: - func(*args, **kwargs) - - sdnotify(b"READY=1") - - # It's okay to ignore the linter error here and call - # `run_as_background_process` directly because `_handle_sighup` operates - # outside of the scope of a specific `HomeServer` instance and holds no - # references to it which would prevent a clean shutdown. - return run_as_background_process( # type: ignore[untracked-background-process] - "sighup", - server_name, - _handle_sighup, - *args, - **kwargs, - ) - - # We defer running the sighup handlers until next reactor tick. This - # is so that we're in a sane state, e.g. flushing the logs may fail - # if the sighup happens in the middle of writing a log entry. - def run_sighup(*args: Any, **kwargs: Any) -> None: - # `callFromThread` should be "signal safe" as well as thread - # safe. - reactor.callFromThread(handle_sighup, *args, **kwargs) - - signal.signal(signal.SIGHUP, run_sighup) - - register_sighup(hs.get_instance_id(), refresh_certificate, hs) - register_sighup(hs.get_instance_id(), reload_cache_config, hs.config) + setup_sighup_handling() + register_sighup(hs.get_instance_id(), refresh_certificate, hs) + register_sighup(hs.get_instance_id(), reload_cache_config, hs.config) # Apply the cache config. hs.config.caches.resize_all_caches()