Merge pull request #4636 from matrix-org/rav/bind_address_fixes

Fix errors when using default bind_addresses with replication/metrics listeners

changelog.d/4636.bugfix

@@ -0,0 +1 @@
+Fix errors when using default bind_addresses with replication/metrics listeners.

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),
+        logger.info("Starting metrics listener on %s:%d", host, port)
-                             addr=host, registry=RegistryProxy)
+        start_http_server(port, addr=host, registry=RegistryProxy)
-        logger.info("Metrics now reporting on %s:%d", host, port)
 
 
 def listen_tcp(bind_addresses, port, factory, reactor=reactor, backlog=50):
@@ -163,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[twisted.internet.tcp.Port]: listening for TCP connections
     """
+    r = []
     for address in bind_addresses:
         try:
-            reactor.listenTCP(
+            r.append(
-                port,
+                reactor.listenTCP(
-                factory,
+                    port,
-                backlog,
+                    factory,
-                address
+                    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 r
-    return []
 
 
 def listen_ssl(
@@ -204,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
 
 
@@ -230,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.

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
@@ -262,14 +266,14 @@ class SynapseHomeServer(HomeServer):
                     )
                 )
             elif listener["type"] == "replication":
-                bind_addresses = listener["bind_addresses"]
+                services = listen_tcp(
-                for address in bind_addresses:
+                    listener["bind_addresses"],
-                    factory = ReplicationStreamProtocolFactory(self)
+                    listener["port"],
-                    server_listener = reactor.listenTCP(
+                    ReplicationStreamProtocolFactory(self),
-                        listener["port"], factory, interface=address
+                )
-                    )
+                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:

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)