From a90bfef4d86f0fb86070ea587fcac8184332cb6b Mon Sep 17 00:00:00 2001 From: mokashang Date: Sat, 12 Sep 2026 13:09:31 -0700 Subject: [PATCH] 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." + )