From e3a0300431e6f641297cb90e936cb2d35fb51850 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Feb 2019 11:48:56 +0000 Subject: [PATCH 1/5] Special-case the default bind_addresses for metrics listener turns out it doesn't really support ipv6, so let's hack around that by only listening on ipv4 by default. --- synapse/app/_base.py | 5 ++--- synapse/config/server.py | 6 +++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 5b0ca312e..1d2c3339f 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -153,9 +153,8 @@ def listen_metrics(bind_addresses, port): from prometheus_client import start_http_server for host in bind_addresses: - reactor.callInThread(start_http_server, int(port), - addr=host, registry=RegistryProxy) - logger.info("Metrics now reporting on %s:%d", host, port) + logger.info("Starting metrics listener on %s:%d", host, port) + start_http_server(port, addr=host, registry=RegistryProxy) def listen_tcp(bind_addresses, port, factory, reactor=reactor, backlog=50): diff --git a/synapse/config/server.py b/synapse/config/server.py index c5c3aac8e..93a30e4cf 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -151,7 +151,11 @@ class ServerConfig(Config): # if we still have an empty list of addresses, use the default list if not bind_addresses: - bind_addresses.extend(DEFAULT_BIND_ADDRESSES) + if listener['type'] == 'metrics': + # the metrics listener doesn't support IPv6 + bind_addresses.append('0.0.0.0') + else: + bind_addresses.extend(DEFAULT_BIND_ADDRESSES) self.listeners.append(listener) From 2a5a15aff8f8d3146df52d5421a6c818637eb629 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Feb 2019 11:53:43 +0000 Subject: [PATCH 2/5] Improve logging around listening services I wanted to bring listen_tcp into line with listen_ssl in terms of returning a list of ports, and wanted to check that was a safe thing to do - hence the logging in `refresh_certificate`. Also, pull the 'Synapse now listening' message up to homeserver.py, because it was being duplicated everywhere else. --- synapse/app/_base.py | 23 ++++++++++++++--------- synapse/app/homeserver.py | 8 ++++++-- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 1d2c3339f..c53b64493 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -162,21 +162,23 @@ def listen_tcp(bind_addresses, port, factory, reactor=reactor, backlog=50): Create a TCP socket for a port and several addresses Returns: - list (empty) + list of twisted.internet.tcp.Port listening for TLS connections """ + r = [] for address in bind_addresses: try: - reactor.listenTCP( - port, - factory, - backlog, - address + r.append( + reactor.listenTCP( + port, + factory, + backlog, + address + ) ) except error.CannotListenError as e: check_bind_error(e, address, bind_addresses) - logger.info("Synapse now listening on TCP port %d", port) - return [] + return r def listen_ssl( @@ -203,7 +205,6 @@ def listen_ssl( except error.CannotListenError as e: check_bind_error(e, address, bind_addresses) - logger.info("Synapse now listening on port %d (TLS)", port) return r @@ -229,6 +230,10 @@ def refresh_certificate(hs): # requests. This factory attribute is public but missing from # Twisted's documentation. if isinstance(i.factory, TLSMemoryBIOFactory): + addr = i.getHost() + logger.info( + "Replacing TLS context factory on [%s]:%i", addr.host, addr.port, + ) # We want to replace TLS factories with a new one, with the new # TLS configuration. We do this by reaching in and pulling out # the wrappedFactory, and then re-wrapping it. diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index dbd98d394..2b008e340 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -121,7 +121,7 @@ class SynapseHomeServer(HomeServer): root_resource = create_resource_tree(resources, root_resource) if tls: - return listen_ssl( + ports = listen_ssl( bind_addresses, port, SynapseSite( @@ -134,9 +134,10 @@ class SynapseHomeServer(HomeServer): self.tls_server_context_factory, reactor=self.get_reactor(), ) + logger.info("Synapse now listening on TCP port %d (TLS)", port) else: - return listen_tcp( + ports = listen_tcp( bind_addresses, port, SynapseSite( @@ -148,6 +149,9 @@ class SynapseHomeServer(HomeServer): ), reactor=self.get_reactor(), ) + logger.info("Synapse now listening on TCP port %d", port) + + return ports def _configure_named_resource(self, name, compress=False): """Build a resource map for a named resource From 767686af48ecbf5fe999830e0b0ce13b7fd46b1b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Feb 2019 11:59:04 +0000 Subject: [PATCH 3/5] Use `listen_tcp` for the replication listener Fixes the "can't listen on 0.0.0.0" error. Also makes it more consistent with what we do elsewhere. --- synapse/app/homeserver.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 2b008e340..dbd9a8387 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -266,14 +266,14 @@ class SynapseHomeServer(HomeServer): ) ) elif listener["type"] == "replication": - bind_addresses = listener["bind_addresses"] - for address in bind_addresses: - factory = ReplicationStreamProtocolFactory(self) - server_listener = reactor.listenTCP( - listener["port"], factory, interface=address - ) + services = listen_tcp( + listener["bind_addresses"], + listener["port"], + ReplicationStreamProtocolFactory(self), + ) + for s in services: reactor.addSystemEventTrigger( - "before", "shutdown", server_listener.stopListening, + "before", "shutdown", s.stopListening, ) elif listener["type"] == "metrics": if not self.get_config().enable_metrics: From 2d0e0a40441f299f06033c20d7ae801100eb0028 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Feb 2019 12:00:34 +0000 Subject: [PATCH 4/5] changelog --- changelog.d/4636.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4636.bugfix diff --git a/changelog.d/4636.bugfix b/changelog.d/4636.bugfix new file mode 100644 index 000000000..7607aa1d5 --- /dev/null +++ b/changelog.d/4636.bugfix @@ -0,0 +1 @@ +Fix errors when using default bind_addresses with replication/metrics listeners. \ No newline at end of file From 309f3bb322ea407b29651bd31c12183ea81b05b1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 13 Feb 2019 13:24:27 +0000 Subject: [PATCH 5/5] Update synapse/app/_base.py Co-Authored-By: richvdh <1389908+richvdh@users.noreply.github.com> --- synapse/app/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index c53b64493..73ca52bd8 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -162,7 +162,7 @@ def listen_tcp(bind_addresses, port, factory, reactor=reactor, backlog=50): Create a TCP socket for a port and several addresses Returns: - list of twisted.internet.tcp.Port listening for TLS connections + list[twisted.internet.tcp.Port]: listening for TCP connections """ r = [] for address in bind_addresses: