From 73ddfba96fe75076f8ea8a40ddcf1d20a038b300 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 14:34:19 -0500 Subject: [PATCH 01/13] Split homeserver creation and setup --- synapse/app/homeserver.py | 50 ++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 3c691906ca1..8dd9c2236ba 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -345,26 +345,17 @@ def load_or_generate_config(argv_options: List[str]) -> HomeServerConfig: return config -def setup( +def create_homeserver( config: HomeServerConfig, reactor: Optional[ISynapseReactor] = None, - freeze: bool = True, ) -> SynapseHomeServer: """ - Create and setup a Synapse homeserver instance given a configuration. + TODO Args: config: The configuration for the homeserver. reactor: Optionally provide a reactor to use. Can be useful in different scenarios that you want control over the reactor, such as tests. - freeze: whether to freeze the homeserver base objects in the garbage collector. - May improve garbage collection performance by marking objects with an effectively - static lifetime as frozen so they don't need to be considered for cleanup. - If you ever want to `shutdown` the homeserver, this needs to be - False otherwise the homeserver cannot be garbage collected after `shutdown`. - - Returns: - A homeserver instance. """ if config.worker.worker_app: @@ -372,7 +363,6 @@ def setup( "You have specified `worker_app` in the config but are attempting to start a non-worker " "instance. Please use `python -m synapse.app.generic_worker` instead (or remove the option if this is the main process)." ) - sys.exit(1) events.USE_FROZEN_DICTS = config.server.use_frozen_dicts synapse.util.caches.TRACK_MEMORY_USAGE = config.caches.track_memory_usage @@ -397,19 +387,42 @@ def setup( ) hs = SynapseHomeServer( - config.server.server_name, + hostname=config.server.server_name, config=config, version_string=f"Synapse/{VERSION}", reactor=reactor, ) - setup_logging(hs, config, use_worker_options=False) + return hs - # Start the tracer - init_tracer(hs) # noqa +def setup( + hs: SynapseHomeServer, + freeze: bool = True, +) -> None: + """ + Create and setup a Synapse homeserver instance given a configuration. + + Args: + hs: The homeserver to setup. + freeze: whether to freeze the homeserver base objects in the garbage collector. + May improve garbage collection performance by marking objects with an effectively + static lifetime as frozen so they don't need to be considered for cleanup. + If you ever want to `shutdown` the homeserver, this needs to be + False otherwise the homeserver cannot be garbage collected after `shutdown`. + + Returns: + A homeserver instance. + """ + + setup_logging(hs, hs.config, use_worker_options=False) + + # Log after we've configured logging. logger.info("Setting up server") + # Start the tracer + init_tracer(hs) # noqa + try: hs.setup() except Exception as e: @@ -428,8 +441,6 @@ async def start() -> None: register_start(hs, start) - return hs - def run(hs: HomeServer) -> None: _base.start_reactor( @@ -449,7 +460,8 @@ def main() -> None: with LoggingContext(name="main", server_name=homeserver_config.server.server_name): # check base requirements check_requirements() - hs = setup(homeserver_config) + hs = create_homeserver(homeserver_config) + setup(hs) # redirect stdio to the logs, if configured. if not hs.config.logging.no_redirect_stdio: From 3521c0efe8a4ffd463a96fa0995db0ea14a01d00 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 15:43:20 -0500 Subject: [PATCH 02/13] Move start related things to `run()` --- synapse/app/homeserver.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 8dd9c2236ba..099d513d673 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -398,18 +398,12 @@ def create_homeserver( def setup( hs: SynapseHomeServer, - freeze: bool = True, ) -> None: """ Create and setup a Synapse homeserver instance given a configuration. Args: hs: The homeserver to setup. - freeze: whether to freeze the homeserver base objects in the garbage collector. - May improve garbage collection performance by marking objects with an effectively - static lifetime as frozen so they don't need to be considered for cleanup. - If you ever want to `shutdown` the homeserver, this needs to be - False otherwise the homeserver cannot be garbage collected after `shutdown`. Returns: A homeserver instance. @@ -428,7 +422,29 @@ def setup( except Exception as e: handle_startup_exception(e) + +def run( + hs: HomeServer, + *, + freeze: bool = True, +) -> None: + """ + Start the reactor and the Synapse homeserver once the reactor is running. + + Args: + hs: The homeserver to run. + freeze: whether to freeze the homeserver base objects in the garbage collector. + May improve garbage collection performance by marking objects with an effectively + static lifetime as frozen so they don't need to be considered for cleanup. + If you ever want to `shutdown` the homeserver, this needs to be + False otherwise the homeserver cannot be garbage collected after `shutdown`. + """ + async def start() -> None: + # TODO: This should be moved to same pattern we use for other background tasks: + # Add to `REQUIRED_ON_BACKGROUND_TASK_STARTUP` and rely on + # `start_background_tasks` to start it. + # # Load the OIDC provider metadatas, if OIDC is enabled. if hs.config.oidc.oidc_enabled: oidc = hs.get_oidc_handler() @@ -439,10 +455,10 @@ async def start() -> None: hs.get_datastores().main.db_pool.updates.start_doing_background_updates() + # Register a callback to be invoked once the reactor is running register_start(hs, start) - -def run(hs: HomeServer) -> None: + # Start the reactor _base.start_reactor( "synapse-homeserver", soft_file_limit=hs.config.server.soft_file_limit, From e65154464d8c2c654352eb916c5fe02694b66b26 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 17:16:02 -0500 Subject: [PATCH 03/13] Rename `run` -> `start` to match start things inside --- synapse/app/homeserver.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 099d513d673..fdae4e6d19e 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -423,7 +423,7 @@ def setup( handle_startup_exception(e) -def run( +def start( hs: HomeServer, *, freeze: bool = True, @@ -440,10 +440,8 @@ def run( False otherwise the homeserver cannot be garbage collected after `shutdown`. """ - async def start() -> None: - # TODO: This should be moved to same pattern we use for other background tasks: - # Add to `REQUIRED_ON_BACKGROUND_TASK_STARTUP` and rely on - # `start_background_tasks` to start it. + async def _start_when_reactor_running() -> None: + # TODO: Feels like this should be moved somewhere else. # # Load the OIDC provider metadatas, if OIDC is enabled. if hs.config.oidc.oidc_enabled: @@ -453,10 +451,13 @@ async def start() -> None: await _base.start(hs, freeze) + # TODO: This should be moved to same pattern we use for other background tasks: + # Add to `REQUIRED_ON_BACKGROUND_TASK_STARTUP` and rely on + # `start_background_tasks` to start it. hs.get_datastores().main.db_pool.updates.start_doing_background_updates() # Register a callback to be invoked once the reactor is running - register_start(hs, start) + register_start(hs, _start_when_reactor_running) # Start the reactor _base.start_reactor( @@ -483,7 +484,7 @@ def main() -> None: if not hs.config.logging.no_redirect_stdio: redirect_stdio_to_logs() - run(hs) + start(hs) if __name__ == "__main__": From 29457a2d217cb8fd0f050dcb464b7f0d1559d53b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 17:18:39 -0500 Subject: [PATCH 04/13] It's better if we do everything in `setup` As we already have the reactor running in the case of Synapse Pro for Small Hosts and `setup()` should handle everything --- synapse/app/homeserver.py | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index fdae4e6d19e..407b474cd87 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -398,12 +398,19 @@ def create_homeserver( def setup( hs: SynapseHomeServer, + *, + freeze: bool = True, ) -> None: """ Create and setup a Synapse homeserver instance given a configuration. Args: hs: The homeserver to setup. + freeze: whether to freeze the homeserver base objects in the garbage collector. + May improve garbage collection performance by marking objects with an effectively + static lifetime as frozen so they don't need to be considered for cleanup. + If you ever want to `shutdown` the homeserver, this needs to be + False otherwise the homeserver cannot be garbage collected after `shutdown`. Returns: A homeserver instance. @@ -422,24 +429,6 @@ def setup( except Exception as e: handle_startup_exception(e) - -def start( - hs: HomeServer, - *, - freeze: bool = True, -) -> None: - """ - Start the reactor and the Synapse homeserver once the reactor is running. - - Args: - hs: The homeserver to run. - freeze: whether to freeze the homeserver base objects in the garbage collector. - May improve garbage collection performance by marking objects with an effectively - static lifetime as frozen so they don't need to be considered for cleanup. - If you ever want to `shutdown` the homeserver, this needs to be - False otherwise the homeserver cannot be garbage collected after `shutdown`. - """ - async def _start_when_reactor_running() -> None: # TODO: Feels like this should be moved somewhere else. # @@ -459,7 +448,16 @@ async def _start_when_reactor_running() -> None: # Register a callback to be invoked once the reactor is running register_start(hs, _start_when_reactor_running) - # Start the reactor + +def start_reactor( + hs: HomeServer, +) -> None: + """ + Start the reactor. + + Args: + hs: The homeserver to run. + """ _base.start_reactor( "synapse-homeserver", soft_file_limit=hs.config.server.soft_file_limit, @@ -484,7 +482,7 @@ def main() -> None: if not hs.config.logging.no_redirect_stdio: redirect_stdio_to_logs() - start(hs) + start_reactor(hs) if __name__ == "__main__": From b1f447868b44bc4690cc297bdccb02084bd034ef Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 17:32:57 -0500 Subject: [PATCH 05/13] Add plans --- synapse/app/homeserver.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 407b474cd87..1d3519a009e 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -440,9 +440,8 @@ async def _start_when_reactor_running() -> None: await _base.start(hs, freeze) - # TODO: This should be moved to same pattern we use for other background tasks: - # Add to `REQUIRED_ON_BACKGROUND_TASK_STARTUP` and rely on - # `start_background_tasks` to start it. + # TODO: This should be moved to `SynapseHomeServer.start_background_tasks` (this + # way it matches the behavior of only running on `main`) hs.get_datastores().main.db_pool.updates.start_doing_background_updates() # Register a callback to be invoked once the reactor is running From 211765fcc2be29a7c8c96be178b4746368e638f1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 17:42:20 -0500 Subject: [PATCH 06/13] Move db_pool `start_doing_background_updates` to `start_background_tasks` --- synapse/app/homeserver.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 1d3519a009e..32a776550f9 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -85,6 +85,13 @@ def gz_wrap(r: Resource) -> Resource: class SynapseHomeServer(HomeServer): DATASTORE_CLASS = DataStore + def start_background_tasks(self) -> None: + super().start_background_tasks() + + # Start any other background tasks needed by Synapse main process. + + self.get_datastores().main.db_pool.updates.start_doing_background_updates() + def _listener_http( self, config: HomeServerConfig, @@ -440,10 +447,6 @@ async def _start_when_reactor_running() -> None: await _base.start(hs, freeze) - # TODO: This should be moved to `SynapseHomeServer.start_background_tasks` (this - # way it matches the behavior of only running on `main`) - hs.get_datastores().main.db_pool.updates.start_doing_background_updates() - # Register a callback to be invoked once the reactor is running register_start(hs, _start_when_reactor_running) From 2cf1f40cb900a07aeffd46b4b54595d08aface64 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 17:43:38 -0500 Subject: [PATCH 07/13] Add docstring --- synapse/app/homeserver.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 32a776550f9..44374c7c63d 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -83,6 +83,10 @@ def gz_wrap(r: Resource) -> Resource: class SynapseHomeServer(HomeServer): + """ + Homeserver class for the main Synapse process. + """ + DATASTORE_CLASS = DataStore def start_background_tasks(self) -> None: From 0967fdf4022f74e68f594ee7b22717d27bb1732b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 17:48:15 -0500 Subject: [PATCH 08/13] Revert "Move db_pool `start_doing_background_updates` to `start_background_tasks`" This reverts commit 211765fcc2be29a7c8c96be178b4746368e638f1. (do this in another PR) --- synapse/app/homeserver.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 44374c7c63d..33453d73ac2 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -89,13 +89,6 @@ class SynapseHomeServer(HomeServer): DATASTORE_CLASS = DataStore - def start_background_tasks(self) -> None: - super().start_background_tasks() - - # Start any other background tasks needed by Synapse main process. - - self.get_datastores().main.db_pool.updates.start_doing_background_updates() - def _listener_http( self, config: HomeServerConfig, @@ -451,6 +444,10 @@ async def _start_when_reactor_running() -> None: await _base.start(hs, freeze) + # TODO: This should be moved to `SynapseHomeServer.start_background_tasks` (this + # way it matches the behavior of only running on `main`) + hs.get_datastores().main.db_pool.updates.start_doing_background_updates() + # Register a callback to be invoked once the reactor is running register_start(hs, _start_when_reactor_running) From 0f50d2695b15541ce8821868ad9a489869d6eaf9 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 3 Oct 2025 17:49:45 -0500 Subject: [PATCH 09/13] Add changelog --- changelog.d/19015.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19015.misc diff --git a/changelog.d/19015.misc b/changelog.d/19015.misc new file mode 100644 index 00000000000..cabc453469b --- /dev/null +++ b/changelog.d/19015.misc @@ -0,0 +1 @@ +Split homeserver creation (`create_homeserver`) and setup (`setup`). From ad3d9812c06201455b8dcfaf98a4b5c0a1ed306c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Oct 2025 17:39:17 -0500 Subject: [PATCH 10/13] Point out what not to do --- synapse/app/homeserver.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 2da40f14192..48382d48f8f 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -443,8 +443,9 @@ async def _start_when_reactor_running() -> None: await _base.start(hs, freeze) - # TODO: This should be moved to `SynapseHomeServer.start_background_tasks` (this - # way it matches the behavior of only running on `main`) + # TODO: This should be moved to `SynapseHomeServer.start_background_tasks` (not + # `HomeServer.start_background_tasks`) (this way it matches the behavior of only + # running on `main`) hs.get_datastores().main.db_pool.updates.start_doing_background_updates() # Register a callback to be invoked once the reactor is running From 18b42fb8bcfc714178dd0dbbfbd8ea205b5b6437 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Oct 2025 17:43:40 -0500 Subject: [PATCH 11/13] Refactor in tests --- tests/app/test_homeserver_start.py | 8 +++++++- tests/config/test_registration_config.py | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/app/test_homeserver_start.py b/tests/app/test_homeserver_start.py index 0d257c98aaa..36a7170d12a 100644 --- a/tests/app/test_homeserver_start.py +++ b/tests/app/test_homeserver_start.py @@ -37,7 +37,13 @@ def test_wrong_start_caught(self) -> None: self.add_lines_to_config([" main:", " host: 127.0.0.1", " port: 1234"]) # Ensure that starting master process with worker config raises an exception with self.assertRaises(ConfigError): + # Do a normal homeserver creation and setup homeserver_config = synapse.app.homeserver.load_or_generate_config( ["-c", self.config_file] ) - synapse.app.homeserver.setup(homeserver_config) + # XXX: The error will be raised at this point + hs = synapse.app.homeserver.create_homeserver(homeserver_config) + # Continue with the setup. We don't expect this to run because we raised + # earlier, but in the future, the code could be refactored to raise the + # error in a different place. + synapse.app.homeserver.setup(hs) diff --git a/tests/config/test_registration_config.py b/tests/config/test_registration_config.py index a8520c91d13..9da0a3f4269 100644 --- a/tests/config/test_registration_config.py +++ b/tests/config/test_registration_config.py @@ -112,7 +112,13 @@ def test_refuse_to_start_if_open_registration_and_no_verification(self) -> None: # Test that allowing open registration without verification raises an error with self.assertRaises(ConfigError): + # Do a normal homeserver creation and setup homeserver_config = synapse.app.homeserver.load_or_generate_config( ["-c", self.config_file] ) - synapse.app.homeserver.setup(homeserver_config) + # XXX: The error will be raised at this point + hs = synapse.app.homeserver.create_homeserver(homeserver_config) + # Continue with the setup. We don't expect this to run because we raised + # earlier, but in the future, the code could be refactored to raise the + # error in a different place. + synapse.app.homeserver.setup(hs) From 32e5f4ba77178adcf25fb092387f82db0b817e20 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Oct 2025 17:50:24 -0500 Subject: [PATCH 12/13] Update docstrings --- synapse/app/homeserver.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 48382d48f8f..eabf74adcd7 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -354,12 +354,15 @@ def create_homeserver( reactor: Optional[ISynapseReactor] = None, ) -> SynapseHomeServer: """ - TODO + Create a homeserver instance for the Synapse main process. Args: config: The configuration for the homeserver. reactor: Optionally provide a reactor to use. Can be useful in different scenarios that you want control over the reactor, such as tests. + + Returns: + A homeserver instance. """ if config.worker.worker_app: @@ -405,7 +408,7 @@ def setup( freeze: bool = True, ) -> None: """ - Create and setup a Synapse homeserver instance given a configuration. + Setup a Synapse homeserver instance given a configuration. Args: hs: The homeserver to setup. From 16c7967b891f6e7efdb047d6147839e07fe2ea2f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 7 Oct 2025 17:53:34 -0500 Subject: [PATCH 13/13] No need to pass in the whole homeserver (just pass homeserver config) --- synapse/app/homeserver.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index eabf74adcd7..6ea412b9f4c 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -456,21 +456,21 @@ async def _start_when_reactor_running() -> None: def start_reactor( - hs: HomeServer, + config: HomeServerConfig, ) -> None: """ - Start the reactor. + Start the reactor (Twisted event-loop). Args: - hs: The homeserver to run. + config: The configuration for the homeserver. """ _base.start_reactor( "synapse-homeserver", - soft_file_limit=hs.config.server.soft_file_limit, - gc_thresholds=hs.config.server.gc_thresholds, - pid_file=hs.config.server.pid_file, - daemonize=hs.config.server.daemonize, - print_pidfile=hs.config.server.print_pidfile, + soft_file_limit=config.server.soft_file_limit, + gc_thresholds=config.server.gc_thresholds, + pid_file=config.server.pid_file, + daemonize=config.server.daemonize, + print_pidfile=config.server.print_pidfile, logger=logger, ) @@ -488,7 +488,7 @@ def main() -> None: if not hs.config.logging.no_redirect_stdio: redirect_stdio_to_logs() - start_reactor(hs) + start_reactor(homeserver_config) if __name__ == "__main__":