Be able to shutdown homeserver that failed to start

changelog.d/19232.misc

@@ -0,0 +1 @@
+Fix `HomeServer.shutdown()` failing if the homeserver failed to `start`.

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
 

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