From 3b11c0b78f1d0b64db33c8e2249a817ef9761c79 Mon Sep 17 00:00:00 2001 From: uditDewan Date: Sat, 11 Jul 2026 16:59:00 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20race=20condition=20in=20?= =?UTF-8?q?=5FIncludedRouter=20lazy=20cache=20rebuild?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- fastapi/routing.py | 78 +++++++++++-------- ...test_included_router_concurrent_rebuild.py | 50 ++++++++++++ 2 files changed, 95 insertions(+), 33 deletions(-) create mode 100644 tests/test_included_router_concurrent_rebuild.py diff --git a/fastapi/routing.py b/fastapi/routing.py index c442b122b..832bf3ea1 100644 --- a/fastapi/routing.py +++ b/fastapi/routing.py @@ -7,6 +7,7 @@ import inspect import json import os import stat +import threading import types from collections.abc import ( AsyncIterator, @@ -1579,49 +1580,60 @@ class _IncludedRouter(BaseRoute): default_factory=list ) _effective_low_priority_routes_version: int | None = None + _rebuild_lock: threading.Lock = field( + default_factory=threading.Lock, repr=False, compare=False + ) def effective_candidates(self) -> list["_EffectiveRouteContext | _IncludedRouter"]: routes_version = self.original_router._get_routes_version() if routes_version == self._effective_candidates_version: return self._effective_candidates - self._effective_candidates = [] - candidates = self.original_router.routes - for route in candidates: - if isinstance(route, _IncludedRouter): - child_context = self.include_context.combine(route.include_context) - child_branch = _IncludedRouter( - original_router=route.original_router, - include_context=child_context, - ) - self._effective_candidates.append(child_branch) - continue - route_context = self._build_effective_context(route) - if route_context is not None: - self._effective_candidates.append(route_context) - self._effective_candidates_version = routes_version - return self._effective_candidates + with self._rebuild_lock: + if routes_version == self._effective_candidates_version: + return self._effective_candidates + effective_candidates: list[_EffectiveRouteContext | _IncludedRouter] = [] + candidates = self.original_router.routes + for route in candidates: + if isinstance(route, _IncludedRouter): + child_context = self.include_context.combine(route.include_context) + child_branch = _IncludedRouter( + original_router=route.original_router, + include_context=child_context, + ) + effective_candidates.append(child_branch) + continue + route_context = self._build_effective_context(route) + if route_context is not None: + effective_candidates.append(route_context) + self._effective_candidates = effective_candidates + self._effective_candidates_version = routes_version + return effective_candidates def effective_low_priority_routes(self) -> list["_EffectiveRouteContext"]: routes_version = self.original_router._get_routes_version() if routes_version == self._effective_low_priority_routes_version: return self._effective_low_priority_routes - self._effective_low_priority_routes = [] - for route in self.original_router._low_priority_routes: - route_context = self._build_effective_context(route) - if route_context is not None: - self._effective_low_priority_routes.append(route_context) - for route in self.original_router.routes: - if isinstance(route, _IncludedRouter): - child_context = self.include_context.combine(route.include_context) - child_branch = _IncludedRouter( - original_router=route.original_router, - include_context=child_context, - ) - self._effective_low_priority_routes.extend( - child_branch.effective_low_priority_routes() - ) - self._effective_low_priority_routes_version = routes_version - return self._effective_low_priority_routes + with self._rebuild_lock: + if routes_version == self._effective_low_priority_routes_version: + return self._effective_low_priority_routes + low_priority_routes: list[_EffectiveRouteContext] = [] + for route in self.original_router._low_priority_routes: + route_context = self._build_effective_context(route) + if route_context is not None: + low_priority_routes.append(route_context) + for route in self.original_router.routes: + if isinstance(route, _IncludedRouter): + child_context = self.include_context.combine(route.include_context) + child_branch = _IncludedRouter( + original_router=route.original_router, + include_context=child_context, + ) + low_priority_routes.extend( + child_branch.effective_low_priority_routes() + ) + self._effective_low_priority_routes = low_priority_routes + self._effective_low_priority_routes_version = routes_version + return low_priority_routes def _build_effective_context( self, route: BaseRoute diff --git a/tests/test_included_router_concurrent_rebuild.py b/tests/test_included_router_concurrent_rebuild.py new file mode 100644 index 000000000..852ee5748 --- /dev/null +++ b/tests/test_included_router_concurrent_rebuild.py @@ -0,0 +1,50 @@ +import sys +import threading + +from fastapi import APIRouter, FastAPI +from fastapi.routing import _IncludedRouter +from fastapi.testclient import TestClient + +N_ROUTES = 120 +N_THREADS = 6 + + +def build_app() -> FastAPI: + app = FastAPI() + router = APIRouter() + for i in range(N_ROUTES): + + def endpoint(i: int = i) -> dict[str, int]: + return {"i": i} + + router.add_api_route(f"/r{i}", endpoint, methods=["GET"]) + app.include_router(router) + return app + + +def test_concurrent_first_requests_do_not_corrupt_candidate_cache() -> None: + app = build_app() + old_interval = sys.getswitchinterval() + sys.setswitchinterval(1e-5) + try: + barrier = threading.Barrier(N_THREADS) + statuses: list[int] = [] + + def worker() -> None: + client = TestClient(app) + barrier.wait() + statuses.append(client.get(f"/r{N_ROUTES - 1}").status_code) + + threads = [threading.Thread(target=worker) for _ in range(N_THREADS)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + finally: + sys.setswitchinterval(old_interval) + + assert statuses == [200] * N_THREADS + included = next( + route for route in app.router.routes if isinstance(route, _IncludedRouter) + ) + assert len(included.effective_candidates()) == N_ROUTES