Be able to shutdown homeserver that failed to start (#19232)
For example, a homeserver can fail to `start` if the port is already in use or the port number is invalid (not 0-65535) Fix https://github.com/element-hq/synapse/issues/19189 Follow-up to https://github.com/element-hq/synapse/pull/18828 ### Background As part of Element's plan to support a light form of vhosting (virtual host) (multiple instances of Synapse in the same Python process) (c.f [Synapse Pro for small hosts](https://docs.element.io/latest/element-server-suite-pro/synapse-pro-for-small-hosts/overview/)), we're currently diving into the details and implications of running multiple instances of Synapse in the same Python process. "Clean tenant deprovisioning" tracked internally by https://github.com/element-hq/synapse-small-hosts/issues/50
This commit is contained in:
1
changelog.d/19232.misc
Normal file
1
changelog.d/19232.misc
Normal file
@@ -0,0 +1 @@
|
|||||||
|
Fix `HomeServer.shutdown()` failing if the homeserver failed to `start`.
|
||||||
@@ -41,7 +41,7 @@ from typing import (
|
|||||||
from wsgiref.simple_server import WSGIServer
|
from wsgiref.simple_server import WSGIServer
|
||||||
|
|
||||||
from cryptography.utils import CryptographyDeprecationWarning
|
from cryptography.utils import CryptographyDeprecationWarning
|
||||||
from typing_extensions import ParamSpec
|
from typing_extensions import ParamSpec, assert_never
|
||||||
|
|
||||||
import twisted
|
import twisted
|
||||||
from twisted.internet import defer, error, reactor as _reactor
|
from twisted.internet import defer, error, reactor as _reactor
|
||||||
@@ -64,7 +64,12 @@ from synapse.app import check_bind_error
|
|||||||
from synapse.config import ConfigError
|
from synapse.config import ConfigError
|
||||||
from synapse.config._base import format_config_error
|
from synapse.config._base import format_config_error
|
||||||
from synapse.config.homeserver import HomeServerConfig
|
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.crypto import context_factory
|
||||||
from synapse.events.auto_accept_invites import InviteAutoAccepter
|
from synapse.events.auto_accept_invites import InviteAutoAccepter
|
||||||
from synapse.events.presence_router import load_legacy_presence_router
|
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(
|
def listen_http(
|
||||||
hs: "HomeServer",
|
hs: "HomeServer",
|
||||||
listener_config: ListenerConfig,
|
listener_config: ListenerConfig,
|
||||||
@@ -447,6 +483,7 @@ def listen_http(
|
|||||||
hs=hs,
|
hs=hs,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
if isinstance(listener_config, TCPListenerConfig):
|
if isinstance(listener_config, TCPListenerConfig):
|
||||||
if listener_config.is_tls():
|
if listener_config.is_tls():
|
||||||
# refresh_certificate should have been called before this.
|
# refresh_certificate should have been called before this.
|
||||||
@@ -468,9 +505,11 @@ def listen_http(
|
|||||||
site,
|
site,
|
||||||
reactor=reactor,
|
reactor=reactor,
|
||||||
)
|
)
|
||||||
logger.info("Synapse now listening on TCP port %d", listener_config.port)
|
logger.info(
|
||||||
|
"Synapse now listening on TCP port %d", listener_config.port
|
||||||
|
)
|
||||||
|
|
||||||
else:
|
elif isinstance(listener_config, UnixListenerConfig):
|
||||||
ports = listen_unix(
|
ports = listen_unix(
|
||||||
listener_config.path, listener_config.mode, site, reactor=reactor
|
listener_config.path, listener_config.mode, site, reactor=reactor
|
||||||
)
|
)
|
||||||
@@ -480,6 +519,19 @@ def listen_http(
|
|||||||
"Synapse now listening on Unix Socket at: %s",
|
"Synapse now listening on Unix Socket at: %s",
|
||||||
ports[0].getHost().name.decode("utf-8"),
|
ports[0].getHost().name.decode("utf-8"),
|
||||||
)
|
)
|
||||||
|
else:
|
||||||
|
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
|
return ports
|
||||||
|
|
||||||
|
|||||||
@@ -815,6 +815,13 @@ class SynapseSite(ProxySite):
|
|||||||
protocol.transport.loseConnection()
|
protocol.transport.loseConnection()
|
||||||
self.connections.clear()
|
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]
|
def log(self, request: SynapseRequest) -> None: # type: ignore[override]
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user