Remove unnecessary reactor reference from `_PerHostRatelimiter` (#14842)

Fix up #14812 to avoid introducing a reference to the reactor.

Signed-off-by: Sean Quah <seanq@matrix.org>

changelog.d/14842.bugfix

@@ -0,0 +1 @@
+Fix a long-standing bug where Synapse would exhaust the stack when processing many federation requests where the remote homeserver has disconencted early.

synapse/rest/client/register.py

@@ -310,7 +310,6 @@ class UsernameAvailabilityRestServlet(RestServlet):
         self.hs = hs
         self.registration_handler = hs.get_registration_handler()
         self.ratelimiter = FederationRateLimiter(
-            hs.get_reactor(),
             hs.get_clock(),
             FederationRatelimitSettings(
                 # Time window of 2s

synapse/server.py

@@ -768,7 +768,6 @@ class HomeServer(metaclass=abc.ABCMeta):
     @cache_in_self
     def get_federation_ratelimiter(self) -> FederationRateLimiter:
         return FederationRateLimiter(
-            self.get_reactor(),
             self.get_clock(),
             config=self.config.ratelimiting.rc_federation,
             metrics_name="federation_servlets",

synapse/util/ratelimitutils.py

@@ -34,7 +34,6 @@ from prometheus_client.core import Counter
 from typing_extensions import ContextManager
 
 from twisted.internet import defer
-from twisted.internet.interfaces import IReactorTime
 
 from synapse.api.errors import LimitExceededError
 from synapse.config.ratelimiting import FederationRatelimitSettings
@@ -147,14 +146,12 @@ class FederationRateLimiter:
 
     def __init__(
         self,
-        reactor: IReactorTime,
         clock: Clock,
         config: FederationRatelimitSettings,
         metrics_name: Optional[str] = None,
     ):
         """
         Args:
-            reactor
             clock
             config
             metrics_name: The name of the rate limiter so we can differentiate it
@@ -166,7 +163,7 @@ class FederationRateLimiter:
 
         def new_limiter() -> "_PerHostRatelimiter":
             return _PerHostRatelimiter(
-                reactor=reactor, clock=clock, config=config, metrics_name=metrics_name
+                clock=clock, config=config, metrics_name=metrics_name
             )
 
         self.ratelimiters: DefaultDict[
@@ -197,14 +194,12 @@ class FederationRateLimiter:
 class _PerHostRatelimiter:
     def __init__(
         self,
-        reactor: IReactorTime,
         clock: Clock,
         config: FederationRatelimitSettings,
         metrics_name: Optional[str] = None,
     ):
         """
         Args:
-            reactor
             clock
             config
             metrics_name: The name of the rate limiter so we can differentiate it
@@ -212,7 +207,6 @@ class _PerHostRatelimiter:
                 for this rate limiter.
                 from the rest in the metrics
         """
-        self.reactor = reactor
         self.clock = clock
         self.metrics_name = metrics_name
 
@@ -388,4 +382,4 @@ class _PerHostRatelimiter:
             except KeyError:
                 pass
 
-        self.reactor.callLater(0.0, start_next_request)
+        self.clock.call_later(0.0, start_next_request)

tests/util/test_ratelimitutils.py

@@ -30,7 +30,7 @@ class FederationRateLimiterTestCase(TestCase):
         """A simple test with the default values"""
         reactor, clock = get_clock()
         rc_config = build_rc_config()
-        ratelimiter = FederationRateLimiter(reactor, clock, rc_config)
+        ratelimiter = FederationRateLimiter(clock, rc_config)
 
         with ratelimiter.ratelimit("testhost") as d1:
             # shouldn't block
@@ -40,7 +40,7 @@ class FederationRateLimiterTestCase(TestCase):
         """Test what happens when we hit the concurrent limit"""
         reactor, clock = get_clock()
         rc_config = build_rc_config({"rc_federation": {"concurrent": 2}})
-        ratelimiter = FederationRateLimiter(reactor, clock, rc_config)
+        ratelimiter = FederationRateLimiter(clock, rc_config)
 
         with ratelimiter.ratelimit("testhost") as d1:
             # shouldn't block
@@ -67,7 +67,7 @@ class FederationRateLimiterTestCase(TestCase):
         rc_config = build_rc_config(
             {"rc_federation": {"sleep_limit": 2, "sleep_delay": 500}}
         )
-        ratelimiter = FederationRateLimiter(reactor, clock, rc_config)
+        ratelimiter = FederationRateLimiter(clock, rc_config)
 
         with ratelimiter.ratelimit("testhost") as d1:
             # shouldn't block
@@ -98,7 +98,7 @@ class FederationRateLimiterTestCase(TestCase):
                 }
             }
         )
-        ratelimiter = FederationRateLimiter(reactor, clock, rc_config)
+        ratelimiter = FederationRateLimiter(clock, rc_config)
 
         with ratelimiter.ratelimit("testhost") as d:
             # shouldn't block