From a90bfef4d86f0fb86070ea587fcac8184332cb6b Mon Sep 17 00:00:00 2001 From: mokashang Date: Sat, 12 Sep 2026 13:09:31 -0700 Subject: [PATCH 1/2] Serialize `Dash._setup_server` under a lock and publish its flag last `_setup_server` runs as a `before_request` hook and sets its `_got_first_request["setup_server"]` guard flag before performing the work that flag protects (populating `registered_paths` via `_generate_scripts_html`, `callback_map` via the `GLOBAL_CALLBACK_MAP` copy, and so on). On a multi-threaded WSGI worker such as `gunicorn -k gthread`, waitress, or `flask run --threaded`, a second request arriving in that gap sees the flag already set, skips setup, then reads `registered_paths` and validates against `callback_map` while both are still empty, so component-bundle requests 500 with `Error loading dependency. "" is not a registered library`. The setup body now runs under a per-instance `threading.Lock` with a double-checked read of the flag: a raced-in thread waits on the lock, then sees the flag set by whichever thread won and returns without redoing the work. The flag is only published once every side effect has been applied, so no other thread can observe it prematurely. Callers on the hot path after the first request pay no lock cost. Adds a regression test in `tests/unit/` that reproduces the race by slowing `_generate_scripts_html` and running several concurrent `_setup_server` calls; before the fix three of four threads returned with `registered_paths` still empty, after the fix all threads see it populated. Closes #3971. --- CHANGELOG.md | 1 + dash/dash.py | 162 +++++++++++++++------------ tests/unit/test_setup_server_race.py | 61 ++++++++++ 3 files changed, 153 insertions(+), 71 deletions(-) create mode 100644 tests/unit/test_setup_server_race.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 51e3212852..3ec1b0662c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). - [#3646](https://github.com/plotly/dash/pull/3646) Remove React 16 support (`16.14.0` is no longer an accepted value for `REACT_VERSION` / `_set_react_version`). ### Fixed +- Fix `Dash._setup_server` publishing its "already done" guard flag before the setup work behind it had run. Under a multi-threaded WSGI worker such as `gunicorn -k gthread` a second request arriving mid-setup could observe the flag already set, skip setup, then read `registered_paths` / `callback_map` while they were still empty, causing the first burst of component bundle requests after a restart to 500 with `Error loading dependency. "" is not a registered library`. The setup body now runs under a lock and only publishes the flag after all work completes. Fixes [#3971](https://github.com/plotly/dash/issues/3971). - [#3944](https://github.com/plotly/dash/pull/3944) Fix `dash.testing` runner backend detection for wrapped FastAPI/Quart servers so threaded Flask-only options are not passed to ASGI runners. - [#3955](https://github.com/plotly/dash/pull/3955) Unpin `selenium` in the testing requirements (was capped at `<=4.2.0`, from 2022) and require `>=4.11.0`. The old cap predated Selenium Manager, so the pinned selenium could not drive the current stable Chrome (151+) that CI installs, producing widespread `StaleElementReferenceException`/`TimeoutException` flakiness across the browser-based integration tests. Modern selenium auto-provisions a matching chromedriver, restoring stable CI runs. - Speed up the layout crawl the renderer runs on every path recompute and callback gather (`crawlLayout` and its callers) by replacing curried-ramda `path`/`pathOr` lookups with direct property access on the per-node hot path. Cut a `Patch().append()` into a large container by ~16% and initial render / wildcard resolution by a few percent, with no behavior change. diff --git a/dash/dash.py b/dash/dash.py index 134e92107f..ae5ee0395b 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -684,6 +684,10 @@ def __init__( # pylint: disable=too-many-statements, too-many-branches # tracks internally if a function already handled at least one request. self._got_first_request = {"pages": False, "setup_server": False} + # Serializes _setup_server so a concurrent worker thread cannot + # observe the guard flag while the setup work behind it is still + # in flight (see gh-3971). + self._setup_server_lock = threading.Lock() # Secret used to sign background-callback handles (see _callback_signing). # Prefer the Flask/Quart secret_key (shared across workers when the @@ -1703,88 +1707,104 @@ def _setup_server(self): if self._got_first_request["setup_server"]: return - self._got_first_request["setup_server"] = True - - # Apply _force_eager_loading overrides from modules - eager_loading = self.config.eager_loading - for module_name in ComponentRegistry.registry: - module = sys.modules[module_name] - eager = getattr(module, "_force_eager_loading", False) - eager_loading = eager_loading or eager - - # Update eager_loading settings - self.scripts.config.eager_loading = eager_loading - - if self.config.include_assets_files: - self._walk_assets_directory() - - if not self.layout and self.use_pages: - self.layout = page_container + # Double-checked locking: previously the guard flag was set before the + # work it protects, which let a second thread (e.g. under gunicorn + # ``-k gthread``) skip setup while ``registered_paths`` and + # ``callback_map`` were still being populated by the first thread and + # then fail validation on component bundle requests. See gh-3971. + with self._setup_server_lock: + if self._got_first_request["setup_server"]: + return - _validate.validate_layout(self.layout, self._layout_value()) + # Apply _force_eager_loading overrides from modules + eager_loading = self.config.eager_loading + for module_name in ComponentRegistry.registry: + module = sys.modules[module_name] + eager = getattr(module, "_force_eager_loading", False) + eager_loading = eager_loading or eager - self._generate_scripts_html() - self._generate_css_dist_html() + # Update eager_loading settings + self.scripts.config.eager_loading = eager_loading - # Copy over global callback data structures assigned with `dash.callback` - for k in list(_callback.GLOBAL_CALLBACK_MAP): - if k in self.callback_map: - raise DuplicateCallback( - f"The callback `{k}` provided with `dash.callback` was already " - "assigned with `app.callback`." - ) + if self.config.include_assets_files: + self._walk_assets_directory() - self.callback_map[k] = _callback.GLOBAL_CALLBACK_MAP.pop(k) + if not self.layout and self.use_pages: + self.layout = page_container - self._callback_list.extend(_callback.GLOBAL_CALLBACK_LIST) + _validate.validate_layout(self.layout, self._layout_value()) - # For each callback function, if the hidden parameter uses the default value None, - # replace it with the actual value of the self.config.hide_all_callbacks. - self._callback_list = [ - ( - {**_callback, "hidden": self.config.get("hide_all_callbacks", False)} - if _callback.get("hidden") is None - else _callback - ) - for _callback in self._callback_list - ] + self._generate_scripts_html() + self._generate_css_dist_html() - _callback.GLOBAL_CALLBACK_LIST.clear() + # Copy over global callback data structures assigned with `dash.callback` + for k in list(_callback.GLOBAL_CALLBACK_MAP): + if k in self.callback_map: + raise DuplicateCallback( + f"The callback `{k}` provided with `dash.callback` was already " + "assigned with `app.callback`." + ) - _validate.validate_background_callbacks(self.callback_map) + self.callback_map[k] = _callback.GLOBAL_CALLBACK_MAP.pop(k) - cancels = {} + self._callback_list.extend(_callback.GLOBAL_CALLBACK_LIST) - for callback in self.callback_map.values(): - background = callback.get("background") - if not background: - continue - if "cancel_inputs" in background: - cancel = background.pop("cancel_inputs") - for c in cancel: - cancels[c] = background.get("manager") - - if cancels: - for cancel_input, manager in cancels.items(): - # pylint: disable=cell-var-from-loop - @self.callback( - Output(cancel_input.component_id, "id"), - cancel_input, - prevent_initial_call=True, - manager=manager, + # For each callback function, if the hidden parameter uses the default value None, + # replace it with the actual value of the self.config.hide_all_callbacks. + self._callback_list = [ + ( + { + **_callback, + "hidden": self.config.get("hide_all_callbacks", False), + } + if _callback.get("hidden") is None + else _callback ) - def cancel_call(*_): - job_ids = callback_context.args.getlist("cancelJob") - executor = _callback.context_value.get().background_callback_manager - if job_ids: - secret = self._get_signing_secret() - end_id = _callback.get_request_end_id(secret) - scope = _callback_signing.job_scope(end_id) - for job_id in job_ids: - job = _callback_signing.unsign(secret, scope, job_id) - if job is not None: - executor.terminate_job(job) - return no_update + for _callback in self._callback_list + ] + + _callback.GLOBAL_CALLBACK_LIST.clear() + + _validate.validate_background_callbacks(self.callback_map) + + cancels = {} + + for callback in self.callback_map.values(): + background = callback.get("background") + if not background: + continue + if "cancel_inputs" in background: + cancel = background.pop("cancel_inputs") + for c in cancel: + cancels[c] = background.get("manager") + + if cancels: + for cancel_input, manager in cancels.items(): + # pylint: disable=cell-var-from-loop + @self.callback( + Output(cancel_input.component_id, "id"), + cancel_input, + prevent_initial_call=True, + manager=manager, + ) + def cancel_call(*_): + job_ids = callback_context.args.getlist("cancelJob") + executor = ( + _callback.context_value.get().background_callback_manager + ) + if job_ids: + secret = self._get_signing_secret() + end_id = _callback.get_request_end_id(secret) + scope = _callback_signing.job_scope(end_id) + for job_id in job_ids: + job = _callback_signing.unsign(secret, scope, job_id) + if job is not None: + executor.terminate_job(job) + return no_update + + # Publish the flag last, so a raced-in thread cannot see it set + # while the setup work above is still in flight. + self._got_first_request["setup_server"] = True def _add_assets_resource(self, url_path, file_path): res = {"asset_path": url_path, "filepath": file_path} diff --git a/tests/unit/test_setup_server_race.py b/tests/unit/test_setup_server_race.py new file mode 100644 index 0000000000..997b0b7253 --- /dev/null +++ b/tests/unit/test_setup_server_race.py @@ -0,0 +1,61 @@ +"""Regression test for gh-3971: ``_setup_server`` TOCTOU race. + +Before the fix, ``Dash._setup_server`` set its guard flag before doing the +work that flag protects (populating ``registered_paths``, ``callback_map``, +etc.). Under a multi-threaded WSGI worker such as ``gunicorn -k gthread``, +a second thread arriving mid-setup could observe the flag already set, skip +setup, and then read ``registered_paths`` while it was still empty, which +caused component-bundle requests to 500 with "Error loading dependency." + +The test simulates a concurrent second request by slowing down one of the +inner setup steps and having several threads call ``_setup_server`` at the +same time. After every thread returns, ``registered_paths`` must be +populated, because a return from ``_setup_server`` is meant to guarantee +the setup work is done. +""" +import threading +import time + +from dash import Dash, html + + +def test_setup_server_is_atomic_under_concurrent_requests(): + app = Dash() + app.layout = html.Div(id="root") + + original_generate_scripts_html = app._generate_scripts_html + + def slow_generate_scripts_html(): + # Widen the TOCTOU window so a stock CPython scheduler reliably lets + # other threads reach the guard while this one is still running. + time.sleep(0.1) + return original_generate_scripts_html() + + app._generate_scripts_html = slow_generate_scripts_html + + thread_count = 4 + barrier = threading.Barrier(thread_count) + paths_seen_after_setup = [] + lock = threading.Lock() + + def worker(): + barrier.wait() + app._setup_server() + with lock: + paths_seen_after_setup.append(set(app.registered_paths)) + + threads = [threading.Thread(target=worker) for _ in range(thread_count)] + for t in threads: + t.start() + for t in threads: + t.join() + + assert len(paths_seen_after_setup) == thread_count + for paths in paths_seen_after_setup: + # Every thread that received control back from _setup_server must + # observe registered_paths already populated by the winning thread. + # Before the fix, threads that raced past the guard saw an empty set. + assert paths, ( + "A thread returned from _setup_server before the setup work " + "was done; registered_paths was still empty." + ) From 6ad48ede77e9aabcc60fb7360bd4dfb5ec3c850b Mon Sep 17 00:00:00 2001 From: mokashang Date: Sun, 20 Sep 2026 09:25:53 -0700 Subject: [PATCH 2/2] Serialize router_sync and router_async under a lock, mirror _setup_server The pages `router_sync` and `router_async` hooks had the same TOCTOU race that gh-3971 hit in `_setup_server`: both set `_got_first_request["pages"]` at the top of the body and only then registered the `_ID_CONTENT` router callback and built `validation_layout`. Under a multi-threaded WSGI worker (or, for the async hook, an ASGI worker with concurrent tasks) a second request arriving mid-setup would see the flag already set, skip the hook, and then serve requests against a callback that was not yet registered. Two workers racing past the guard would both try to register the same output and hit `DuplicateCallback`. The sync hook now runs under a per-instance `threading.Lock` with a double-checked read of the flag, matching `_setup_server`. The async hook uses an `asyncio.Lock`, created inside `router_async` on the first call so it binds to the running event loop (pre-3.10 `asyncio.Lock` captures the current loop at construction, which for a Dash instance built in module scope is the wrong one). Both hooks publish the flag last, so a raced-in worker cannot observe it while the callback registration and validation-layout build are still pending. Adds `tests/unit/test_pages_router_race.py` with a regression test per hook. Each widens the TOCTOU window inside the guarded region and snapshots the app state at the moment each caller returns; before the fix the raced-in caller returned with the router callback unregistered or `validation_layout` unbuilt. Also renames the CHANGELOG entry for this PR to lead with `[#3980](https://github.com/plotly/dash/pull/3980)` so it matches the convention of the neighbouring entries. Refs #3971. --- CHANGELOG.md | 2 +- dash/dash.py | 292 +++++++++++++++------------ tests/unit/test_pages_router_race.py | 159 +++++++++++++++ 3 files changed, 323 insertions(+), 130 deletions(-) create mode 100644 tests/unit/test_pages_router_race.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ec1b0662c..7ac79f0871 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). - [#3646](https://github.com/plotly/dash/pull/3646) Remove React 16 support (`16.14.0` is no longer an accepted value for `REACT_VERSION` / `_set_react_version`). ### Fixed -- Fix `Dash._setup_server` publishing its "already done" guard flag before the setup work behind it had run. Under a multi-threaded WSGI worker such as `gunicorn -k gthread` a second request arriving mid-setup could observe the flag already set, skip setup, then read `registered_paths` / `callback_map` while they were still empty, causing the first burst of component bundle requests after a restart to 500 with `Error loading dependency. "" is not a registered library`. The setup body now runs under a lock and only publishes the flag after all work completes. Fixes [#3971](https://github.com/plotly/dash/issues/3971). +- [#3980](https://github.com/plotly/dash/pull/3980) Fix the three `before_request` hooks (`Dash._setup_server` and the pages `router_sync` / `router_async`) publishing their "already done" guard flag before the setup work behind it had run. Under a multi-threaded WSGI worker such as `gunicorn -k gthread` (or under an ASGI worker for the async router), a second request arriving mid-setup could observe the flag already set, skip setup, then read `registered_paths` / `callback_map` / the pages router callback while they were still being registered - causing the first burst of component bundle requests after a restart to 500 with `Error loading dependency. "" is not a registered library`, or the pages router to hit `DuplicateCallback` when two workers raced past the guard. Each hook body now runs under a lock (`threading.Lock` for the two sync hooks, an `asyncio.Lock` bound to the running loop for the async router) and only publishes the flag after all work completes. Fixes [#3971](https://github.com/plotly/dash/issues/3971). - [#3944](https://github.com/plotly/dash/pull/3944) Fix `dash.testing` runner backend detection for wrapped FastAPI/Quart servers so threaded Flask-only options are not passed to ASGI runners. - [#3955](https://github.com/plotly/dash/pull/3955) Unpin `selenium` in the testing requirements (was capped at `<=4.2.0`, from 2022) and require `>=4.11.0`. The old cap predated Selenium Manager, so the pinned selenium could not drive the current stable Chrome (151+) that CI installs, producing widespread `StaleElementReferenceException`/`TimeoutException` flakiness across the browser-based integration tests. Modern selenium auto-provisions a matching chromedriver, restoring stable CI runs. - Speed up the layout crawl the renderer runs on every path recompute and callback gather (`crawlLayout` and its callers) by replacing curried-ramda `path`/`pathOr` lookups with direct property access on the per-node hot path. Cut a `Patch().append()` into a large container by ~16% and initial render / wildcard resolution by a few percent, with no behavior change. diff --git a/dash/dash.py b/dash/dash.py index ae5ee0395b..a5ee12a873 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -1,3 +1,4 @@ +import asyncio import functools import os import sys @@ -684,10 +685,15 @@ def __init__( # pylint: disable=too-many-statements, too-many-branches # tracks internally if a function already handled at least one request. self._got_first_request = {"pages": False, "setup_server": False} - # Serializes _setup_server so a concurrent worker thread cannot - # observe the guard flag while the setup work behind it is still - # in flight (see gh-3971). + # Serialize the before_request hooks so a raced-in worker cannot see + # the guard flag while the work behind it is still in flight (gh-3971). + # The async lock is created inside router_async so it binds to the + # running event loop; on 3.9 asyncio.Lock captures the loop at + # construction, which for a Dash instance built in module scope is + # the wrong one. self._setup_server_lock = threading.Lock() + self._pages_lock = threading.Lock() + self._pages_async_lock: Optional[asyncio.Lock] = None # Secret used to sign background-callback handles (see _callback_signing). # Prefer the Flask/Quart secret_key (shared across workers when the @@ -2657,153 +2663,181 @@ def enable_pages(self) -> None: async def router_async(): if self._got_first_request["pages"]: return - self._got_first_request["pages"] = True - inputs = { - "pathname_": Input(_ID_LOCATION, "pathname"), - "search_": Input(_ID_LOCATION, "search"), - } - inputs.update(self.routing_callback_inputs) - - @self.callback( - Output(_ID_CONTENT, "children"), - Output(_ID_STORE, "data"), - inputs=inputs, - prevent_initial_call=True, - hidden=True, - ) - async def update(pathname_, search_, **states): - query_parameters = _parse_query_string(search_) - page, path_variables = _path_to_page( - self.strip_relative_path(pathname_) + # Lazily create the lock so it binds to the running event loop + # (see __init__). Check-and-assign is safe here: asyncio only + # yields at await points, so no other task can race between the + # two lines. + if self._pages_async_lock is None: + self._pages_async_lock = asyncio.Lock() + + # Double-checked locking, same rationale as _setup_server + # (gh-3971). Without it a raced-in task can either serve requests + # against an unregistered _ID_CONTENT callback or hit + # DuplicateCallback registering the router callback twice. + async with self._pages_async_lock: + if self._got_first_request["pages"]: + return + + inputs = { + "pathname_": Input(_ID_LOCATION, "pathname"), + "search_": Input(_ID_LOCATION, "search"), + } + inputs.update(self.routing_callback_inputs) + + @self.callback( + Output(_ID_CONTENT, "children"), + Output(_ID_STORE, "data"), + inputs=inputs, + prevent_initial_call=True, + hidden=True, ) - if page == {}: - for module, page in _pages.PAGE_REGISTRY.items(): - if module.split(".")[-1] == "not_found_404": - layout = page["layout"] - title = page["title"] - break + async def update(pathname_, search_, **states): + query_parameters = _parse_query_string(search_) + page, path_variables = _path_to_page( + self.strip_relative_path(pathname_) + ) + if page == {}: + for module, page in _pages.PAGE_REGISTRY.items(): + if module.split(".")[-1] == "not_found_404": + layout = page["layout"] + title = page["title"] + break + else: + layout = html.H1("404 - Page not found") + title = self.title else: - layout = html.H1("404 - Page not found") - title = self.title - else: - layout = page.get("layout", "") - title = page["title"] + layout = page.get("layout", "") + title = page["title"] - if callable(layout): - layout = await execute_async_function( - layout, - **{**(path_variables or {}), **query_parameters, **states}, - ) - if callable(title): - title = await execute_async_function( - title, **{**(path_variables or {})} - ) - return layout, {"title": title} + if callable(layout): + layout = await execute_async_function( + layout, + **{**(path_variables or {}), **query_parameters, **states}, + ) + if callable(title): + title = await execute_async_function( + title, **{**(path_variables or {})} + ) + return layout, {"title": title} - _validate.check_for_duplicate_pathnames(_pages.PAGE_REGISTRY) - _validate.validate_registry(_pages.PAGE_REGISTRY) + _validate.check_for_duplicate_pathnames(_pages.PAGE_REGISTRY) + _validate.validate_registry(_pages.PAGE_REGISTRY) - if not self.config.suppress_callback_exceptions: + if not self.config.suppress_callback_exceptions: - async def get_layouts(): - return [ - await execute_async_function(page["layout"]) - if callable(page["layout"]) - else page["layout"] - for page in _pages.PAGE_REGISTRY.values() - ] + async def get_layouts(): + return [ + await execute_async_function(page["layout"]) + if callable(page["layout"]) + else page["layout"] + for page in _pages.PAGE_REGISTRY.values() + ] - layouts = await get_layouts() - # pylint: disable=not-callable - layouts += [self.layout() if callable(self.layout) else self.layout] - self.validation_layout = html.Div(layouts) - if _ID_CONTENT not in self.validation_layout: - raise Exception("`dash.page_container` not found in the layout") - - self.clientside_callback( - """ - function(data) { - document.title = data.title - } - """, - Output(_ID_DUMMY, "children"), - Input(_ID_STORE, "data"), - hidden=True, - ) + layouts = await get_layouts() + # pylint: disable=not-callable + layouts += [self.layout() if callable(self.layout) else self.layout] + self.validation_layout = html.Div(layouts) + if _ID_CONTENT not in self.validation_layout: + raise Exception("`dash.page_container` not found in the layout") + + self.clientside_callback( + """ + function(data) { + document.title = data.title + } + """, + Output(_ID_DUMMY, "children"), + Input(_ID_STORE, "data"), + hidden=True, + ) + + # Publish the flag last so a raced-in task cannot observe it + # set while the callback registration above is still pending. + self._got_first_request["pages"] = True # Sync version def router_sync(): if self._got_first_request["pages"]: return - self._got_first_request["pages"] = True - inputs = { - "pathname_": Input(_ID_LOCATION, "pathname"), - "search_": Input(_ID_LOCATION, "search"), - } - inputs.update(self.routing_callback_inputs) - - @self.callback( - Output(_ID_CONTENT, "children"), - Output(_ID_STORE, "data"), - inputs=inputs, - prevent_initial_call=True, - hidden=True, - ) - def update(pathname_, search_, **states): - query_parameters = _parse_query_string(search_) - page, path_variables = _path_to_page( - self.strip_relative_path(pathname_) + # Double-checked locking, same rationale as router_async and + # _setup_server (gh-3971). + with self._pages_lock: + if self._got_first_request["pages"]: + return + + inputs = { + "pathname_": Input(_ID_LOCATION, "pathname"), + "search_": Input(_ID_LOCATION, "search"), + } + inputs.update(self.routing_callback_inputs) + + @self.callback( + Output(_ID_CONTENT, "children"), + Output(_ID_STORE, "data"), + inputs=inputs, + prevent_initial_call=True, + hidden=True, ) - if page == {}: - for module, page in _pages.PAGE_REGISTRY.items(): - if module.split(".")[-1] == "not_found_404": - layout = page["layout"] - title = page["title"] - break + def update(pathname_, search_, **states): + query_parameters = _parse_query_string(search_) + page, path_variables = _path_to_page( + self.strip_relative_path(pathname_) + ) + if page == {}: + for module, page in _pages.PAGE_REGISTRY.items(): + if module.split(".")[-1] == "not_found_404": + layout = page["layout"] + title = page["title"] + break + else: + layout = html.H1("404 - Page not found") + title = self.title else: - layout = html.H1("404 - Page not found") - title = self.title - else: - layout = page.get("layout", "") - title = page["title"] + layout = page.get("layout", "") + title = page["title"] - if callable(layout): - layout = layout( - **{**(path_variables or {}), **query_parameters, **states} + if callable(layout): + layout = layout( + **{**(path_variables or {}), **query_parameters, **states} + ) + if callable(title): + title = title(**(path_variables or {})) + return layout, {"title": title} + + _validate.check_for_duplicate_pathnames(_pages.PAGE_REGISTRY) + _validate.validate_registry(_pages.PAGE_REGISTRY) + + if not self.config.suppress_callback_exceptions: + layout = self.layout + if not isinstance(layout, list): + # pylint: disable=not-callable + layout = [self.layout() if callable(self.layout) else self.layout] + self.validation_layout = html.Div( + [ + page["layout"]() if callable(page["layout"]) else page["layout"] + for page in _pages.PAGE_REGISTRY.values() + ] + + layout ) - if callable(title): - title = title(**(path_variables or {})) - return layout, {"title": title} + if _ID_CONTENT not in self.validation_layout: + raise Exception("`dash.page_container` not found in the layout") - _validate.check_for_duplicate_pathnames(_pages.PAGE_REGISTRY) - _validate.validate_registry(_pages.PAGE_REGISTRY) - - if not self.config.suppress_callback_exceptions: - layout = self.layout - if not isinstance(layout, list): - # pylint: disable=not-callable - layout = [self.layout() if callable(self.layout) else self.layout] - self.validation_layout = html.Div( - [ - page["layout"]() if callable(page["layout"]) else page["layout"] - for page in _pages.PAGE_REGISTRY.values() - ] - + layout + self.clientside_callback( + """ + function(data) { + document.title = data.title + } + """, + Output(_ID_DUMMY, "children"), + Input(_ID_STORE, "data"), ) - if _ID_CONTENT not in self.validation_layout: - raise Exception("`dash.page_container` not found in the layout") - self.clientside_callback( - """ - function(data) { - document.title = data.title - } - """, - Output(_ID_DUMMY, "children"), - Input(_ID_STORE, "data"), - ) + # Publish the flag last so a raced-in thread cannot observe + # it set while the callback registration above is still + # pending. + self._got_first_request["pages"] = True if self._use_async: self.backend.before_request(router_async) diff --git a/tests/unit/test_pages_router_race.py b/tests/unit/test_pages_router_race.py new file mode 100644 index 0000000000..6c0cfaaeca --- /dev/null +++ b/tests/unit/test_pages_router_race.py @@ -0,0 +1,159 @@ +"""Regression test for gh-3971: pages ``router_sync`` / ``router_async`` TOCTOU race. + +Both hooks used to set ``_got_first_request["pages"]`` at their top and only +then register the ``_ID_CONTENT`` router callback and build +``validation_layout``. Under a multi-threaded WSGI worker (for the sync +hook) or an ASGI worker with multiple concurrent tasks (for the async +one), a second request arriving mid-setup could observe the flag already +set, skip setup, then serve a request against a callback that was not +yet registered. + +The tests here reproduce the window by pausing setup inside the guarded +region and driving the hook concurrently. The state observed at the +moment each caller returns must reflect a fully completed setup. +""" + +import asyncio +import threading +import time + +import dash +from dash import Dash, dependencies, html + +ROUTER_OUTPUT_ID = "_pages_content" + + +def _grab_before_request(app): + """Capture the ``before_request`` closure that ``enable_pages`` installs.""" + captured = {} + original = app.backend.before_request + + def capture(fn): + captured["fn"] = fn + + app.backend.before_request = capture # type: ignore[method-assign] + try: + app.enable_pages() + finally: + app.backend.before_request = original # type: ignore[method-assign] + return captured["fn"] + + +def test_router_sync_is_atomic_under_concurrent_requests(clear_pages_state): + app = Dash(use_pages=True, pages_folder="") + dash.register_page("home_page", path="/", layout=html.Div("home")) + app.layout = html.Div([dash.page_container]) + app._use_async = False + + original_input_init = dependencies.Input.__init__ + slowed = {"first": True} + slow_lock = threading.Lock() + + def slow_input_init(self, component_id, component_property): + # Widen the window between "flag is set" and "router callback is + # registered" so a stock CPython scheduler reliably lets other + # threads through the pages guard while the winning thread is + # still building its Input arguments. + with slow_lock: + need_sleep = slowed["first"] + slowed["first"] = False + if need_sleep: + time.sleep(0.1) + original_input_init(self, component_id, component_property) + + dependencies.Input.__init__ = slow_input_init # type: ignore[assignment] + try: + router_sync = _grab_before_request(app) + + thread_count = 4 + barrier = threading.Barrier(thread_count) + callback_maps_seen = [] + errors = [] + lock = threading.Lock() + + def worker(): + barrier.wait() + try: + router_sync() + except Exception as e: # noqa: BLE001 + with lock: + errors.append(e) + return + # Snapshot ``callback_map`` at the moment this thread's call + # returns. A raced-in thread that skipped setup on the old + # (buggy) code path would return here with the map empty of + # the router callback. + with lock: + callback_maps_seen.append(dict(app.callback_map)) + + threads = [threading.Thread(target=worker) for _ in range(thread_count)] + for t in threads: + t.start() + for t in threads: + t.join() + finally: + dependencies.Input.__init__ = original_input_init # type: ignore[assignment] + + assert not errors, f"router_sync raised under concurrent callers: {errors}" + assert len(callback_maps_seen) == thread_count + for cm in callback_maps_seen: + router_entries = [k for k in cm if ROUTER_OUTPUT_ID in k] + assert router_entries, ( + "A thread returned from router_sync before the router callback " + "was registered; callback_map was still missing the entry. " + "(Before the fix, threads that raced past the guard would see " + "the flag set but no callback yet.)" + ) + winning_map = callback_maps_seen[-1] + router_entries = [k for k in winning_map if ROUTER_OUTPUT_ID in k] + assert len(router_entries) == 1 + + +def test_router_async_is_atomic_under_concurrent_tasks(clear_pages_state): + async def slow_home_layout(): + # Force an await inside the guarded region (via ``get_layouts()``) + # so a concurrent task can slip past the pages guard while this + # one is still building ``validation_layout``. + await asyncio.sleep(0.05) + return html.Div("home") + + app = Dash(use_pages=True, pages_folder="") + dash.register_page("home_page", path="/", layout=slow_home_layout) + app.layout = html.Div([dash.page_container]) + app._use_async = True + + router_async = _grab_before_request(app) + + async def wrapped(): + await router_async() + # Snapshot the state at the moment this task's call returns. + # Before the fix, a task that raced past the guard would return + # here with ``validation_layout`` still unset. + return ( + [k for k in app.callback_map if ROUTER_OUTPUT_ID in k], + getattr(app, "validation_layout", None), + ) + + async def run_two_racing_tasks(): + return await asyncio.gather(wrapped(), wrapped(), return_exceptions=True) + + results = asyncio.run(run_two_racing_tasks()) + + for r in results: + assert not isinstance( + r, Exception + ), f"router_async raised under concurrent callers: {r!r}" + + for router_entries, vlayout in results: + assert router_entries, ( + "A task returned from router_async before the router callback " + "was registered." + ) + assert vlayout is not None, ( + "A task returned from router_async before validation_layout " + "was built. Before the fix, a raced-in task would return here " + "while the winning task was still awaiting ``get_layouts()``." + ) + + router_entries = [k for k in app.callback_map if ROUTER_OUTPUT_ID in k] + assert len(router_entries) == 1