diff --git a/changelog.d/19232.misc b/changelog.d/19232.misc new file mode 100644 index 00000000000..6e3e2ff6494 --- /dev/null +++ b/changelog.d/19232.misc @@ -0,0 +1 @@ +Fix `HomeServer.shutdown()` failing if the homeserver failed to `start`. diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 52bdb9e0d71..d1ed1201e5a 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -41,7 +41,7 @@ from wsgiref.simple_server import WSGIServer from cryptography.utils import CryptographyDeprecationWarning -from typing_extensions import ParamSpec +from typing_extensions import ParamSpec, assert_never import twisted from twisted.internet import defer, error, reactor as _reactor @@ -64,7 +64,12 @@ from synapse.config import ConfigError from synapse.config._base import format_config_error from synapse.config.homeserver import HomeServerConfig -from synapse.config.server import ListenerConfig, ManholeConfig, TCPListenerConfig +from synapse.config.server import ( + ListenerConfig, + ManholeConfig, + TCPListenerConfig, + UnixListenerConfig, +) from synapse.crypto import context_factory from synapse.events.auto_accept_invites import InviteAutoAccepter from synapse.events.presence_router import load_legacy_presence_router @@ -413,6 +418,37 @@ def listen_unix( ] +class ListenerException(RuntimeError): + """ + An exception raised when we fail to listen with the given `ListenerConfig`. + + Attributes: + listener_config: The listener config that caused the exception. + """ + + def __init__( + self, + listener_config: ListenerConfig, + ): + listener_human_name = "" + port = "" + if isinstance(listener_config, TCPListenerConfig): + listener_human_name = "TCP port" + port = str(listener_config.port) + elif isinstance(listener_config, UnixListenerConfig): + listener_human_name = "unix socket" + port = listener_config.path + else: + assert_never(listener_config) + + super().__init__( + "Failed to listen on %s (%s) with the given listener config: %s" + % (listener_human_name, port, listener_config) + ) + + self.listener_config = listener_config + + def listen_http( hs: "HomeServer", listener_config: ListenerConfig, @@ -447,39 +483,55 @@ def listen_http( hs=hs, ) - if isinstance(listener_config, TCPListenerConfig): - if listener_config.is_tls(): - # refresh_certificate should have been called before this. - assert context_factory is not None - ports = listen_ssl( - listener_config.bind_addresses, - listener_config.port, - site, - context_factory, - reactor=reactor, + try: + if isinstance(listener_config, TCPListenerConfig): + if listener_config.is_tls(): + # refresh_certificate should have been called before this. + assert context_factory is not None + ports = listen_ssl( + listener_config.bind_addresses, + listener_config.port, + site, + context_factory, + reactor=reactor, + ) + logger.info( + "Synapse now listening on TCP port %d (TLS)", listener_config.port + ) + else: + ports = listen_tcp( + listener_config.bind_addresses, + listener_config.port, + site, + reactor=reactor, + ) + logger.info( + "Synapse now listening on TCP port %d", listener_config.port + ) + + elif isinstance(listener_config, UnixListenerConfig): + ports = listen_unix( + listener_config.path, listener_config.mode, site, reactor=reactor ) + # getHost() returns a UNIXAddress which contains an instance variable of 'name' + # encoded as a byte string. Decode as utf-8 so pretty. logger.info( - "Synapse now listening on TCP port %d (TLS)", listener_config.port + "Synapse now listening on Unix Socket at: %s", + ports[0].getHost().name.decode("utf-8"), ) else: - ports = listen_tcp( - listener_config.bind_addresses, - listener_config.port, - site, - reactor=reactor, - ) - logger.info("Synapse now listening on TCP port %d", listener_config.port) - - else: - ports = listen_unix( - listener_config.path, listener_config.mode, site, reactor=reactor - ) - # getHost() returns a UNIXAddress which contains an instance variable of 'name' - # encoded as a byte string. Decode as utf-8 so pretty. - logger.info( - "Synapse now listening on Unix Socket at: %s", - ports[0].getHost().name.decode("utf-8"), - ) + assert_never(listener_config) + except Exception as exc: + # The Twisted interface says that "Users should not call this function + # themselves!" but this appears to be the correct/only way handle proper cleanup + # of the site when things go wrong. In the normal case, a `Port` is created + # which we can call `Port.stopListening()` on to do the same thing (but no + # `Port` is created when an error occurs). + # + # We use `site.stopFactory()` instead of `site.doStop()` as the latter assumes + # that `site.doStart()` was called (which won't be the case if an error occurs). + site.stopFactory() + raise ListenerException(listener_config) from exc return ports diff --git a/synapse/http/site.py b/synapse/http/site.py index 03d5d048b11..a1b0b8d9c2f 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -815,6 +815,13 @@ def stopFactory(self) -> None: protocol.transport.loseConnection() self.connections.clear() + # Replace the resource tree with an empty resource to break circular references + # to the resource tree which holds a bunch of homeserver references. This is + # important if we try to call `hs.shutdown()` after `start` fails. For some + # reason, this doesn't seem to be necessary in the normal case where `start` + # succeeds and we call `hs.shutdown()` later. + self.resource = Resource() + def log(self, request: SynapseRequest) -> None: # type: ignore[override] pass