Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/reflex-base/news/6933.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Loading `rxconfig.py` no longer swaps out `sys.path` for the duration of the import, which made concurrent first-time imports in other threads fail with `ModuleNotFoundError` (e.g. the lazy `granian` import while the backend starts). The cwd is now prepended and removed afterwards, and a stale `rxconfig` module from a previously loaded project directory no longer fakes the existence check.
88 changes: 65 additions & 23 deletions packages/reflex-base/src/reflex_base/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -808,34 +808,79 @@ def _set_persistent(self, **kwargs):
_config_module_deps: set[str] = set()


class _ImportRecorder:
"""Meta-path finder that records import attempts on the recording thread.

Never resolves anything; per-thread recording keeps concurrent imports by
other threads out of the rxconfig dep set (unlike a sys.modules diff).
Left in sys.meta_path permanently: removal shifts the list under other
threads' unlocked _find_spec iteration, which can skip a real finder, and
raises ValueError if rxconfig.py rebuilt sys.meta_path.
"""

def __init__(self) -> None:
"""Initialize the recorder as inactive."""
self._thread: int | None = None
self.names: set[str] = set()

def start(self) -> None:
"""Start recording imports made on the current thread."""
self.names.clear()
self._thread = threading.get_ident()

def stop(self) -> None:
"""Stop recording; names stay readable."""
self._thread = None

def find_spec(self, fullname: str, path: Any = None, target: Any = None) -> None:
"""Record the import attempt without resolving it.

Args:
fullname: The module being imported.
path: Unused.
target: Unused.
"""
if threading.get_ident() == self._thread:
self.names.add(fullname)


_import_recorder = _ImportRecorder()


def _get_config() -> Config:
"""Import rxconfig.py fresh and return its config object.

Returns:
The app config.
"""
# Never cache rxconfig or its project-local dependencies — each load goes
# to disk so different RegistrationContexts hold independent Config
# instances resolved against the current project. Evict before probing:
# find_spec answers from sys.modules, so a leftover module from another
# project directory would fake the existence check below.
sys.modules.pop(constants.Config.MODULE, None)
# only import the module if it exists. If a module spec exists then
# the module exists.
spec = find_spec(constants.Config.MODULE)
if not spec:
# we need this condition to ensure that a ModuleNotFound error is not thrown when
# running unit/integration tests or during `reflex init`.
return Config(app_name="", _skip_plugins_checks=True)
# Never cache rxconfig or its project-local dependencies — each load goes
# to disk so different RegistrationContexts hold independent Config
# instances resolved against the current project.
sys.modules.pop(constants.Config.MODULE, None)
for dep in _config_module_deps:
sys.modules.pop(dep, None)
_config_module_deps.clear()
before = set(sys.modules)
# Reinstall if rxconfig.py rebuilt sys.meta_path on a previous load.
if _import_recorder not in sys.meta_path:
sys.meta_path.insert(0, _import_recorder)
_import_recorder.start()
try:
rxconfig = importlib.import_module(constants.Config.MODULE)
finally:
_import_recorder.stop()
# Record even on failure so a retry evicts partially-imported deps.
project_root = Path.cwd()
for name in set(sys.modules) - before:
origin = getattr(sys.modules[name], "__file__", None)
for name in _import_recorder.names:
origin = getattr(sys.modules.get(name), "__file__", None)
if (
origin
and (path := Path(origin)).is_relative_to(project_root)
Expand Down Expand Up @@ -872,30 +917,27 @@ def get_state_auto_setters() -> bool:


def _load_config() -> Config:
"""Load the config from rxconfig.py with cwd on sys.path.
"""Load the config from rxconfig.py with cwd prepended to sys.path.

Prepending (not replacing sys.path) keeps concurrent imports in other
threads working while rxconfig resolves from the app directory first.

Returns:
The app config.
"""
with _load_config_lock:
orig_sys_path = sys.path.copy()
sys.path.clear()
sys.path.append(str(Path.cwd()))
# A fresh str object, so the exact inserted entry can be removed by
# identity: rxconfig.py may itself add or remove equal cwd entries,
# which removal by value could confuse with caller-owned ones.
cwd = str(Path.cwd())
sys.path.insert(0, cwd)
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
try:
return _get_config()
except Exception:
# If the module import fails, try to import with the original sys.path.
sys.path.extend(orig_sys_path)
return _get_config()
finally:
# Find any entries added to sys.path by rxconfig.py itself.
extra_paths = [
p for p in sys.path if p not in orig_sys_path and p != str(Path.cwd())
]
# Restore the original sys.path.
sys.path.clear()
sys.path.extend(extra_paths)
sys.path.extend(orig_sys_path)
for i, entry in enumerate(sys.path):
if entry is cwd:
del sys.path[i]
break


def get_config() -> Config:
Expand Down
163 changes: 163 additions & 0 deletions tests/units/test_config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import multiprocessing
import os
import sys
import threading
import time
from pathlib import Path
Expand Down Expand Up @@ -890,3 +891,165 @@ def worker(i: int) -> None:

assert load_count == 1
assert all(config is results[0] for config in results)


def test_load_config_keeps_sys_path_usable_for_other_threads(
Comment thread
greptile-apps[bot] marked this conversation as resolved.
monkeypatch: pytest.MonkeyPatch,
):
"""Importing an unrelated module while rxconfig loads must succeed.

_load_config used to clear sys.path down to the cwd for the duration of
the rxconfig import, so any concurrent first-time import in another
thread (e.g. the lazy granian import when the backend starts) failed
with ModuleNotFoundError.

Args:
monkeypatch: The pytest monkeypatch fixture.
"""
inside_load = threading.Event()
release_load = threading.Event()

def blocking_get_config() -> rx.Config:
inside_load.set()
release_load.wait(timeout=5)
return rx.Config(app_name="racer")

monkeypatch.setattr(reflex_base.config, "_get_config", blocking_get_config)
# A stdlib module that nothing imports by default; drop it so the import
# below walks sys.path again.
sys.modules.pop("colorsys", None)
sys_path_before = sys.path.copy()

loader = threading.Thread(target=reflex_base.config._load_config)
loader.start()
try:
assert inside_load.wait(timeout=5)
import colorsys # noqa: F401
finally:
release_load.set()
loader.join(timeout=5)
assert not loader.is_alive()
# The temporarily prepended cwd entry was removed again.
assert sys.path == sys_path_before


def test_load_config_keeps_caller_owned_cwd_entry(monkeypatch: pytest.MonkeyPatch):
"""A pre-existing cwd entry survives even if rxconfig removes one itself.

The cleanup must only take back the entry _load_config prepended, not a
caller-owned equal entry.

Args:
monkeypatch: The pytest monkeypatch fixture.
"""
cwd = str(Path.cwd())
monkeypatch.setattr(sys, "path", [cwd, *sys.path])
caller_owned = sys.path.count(cwd)

def removing_get_config() -> rx.Config:
sys.path.remove(cwd)
return rx.Config(app_name="pathological")

monkeypatch.setattr(reflex_base.config, "_get_config", removing_get_config)
reflex_base.config._load_config()
assert sys.path.count(cwd) == caller_owned


def test_concurrent_import_not_recorded_as_rxconfig_dep(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
):
"""A project-local module imported by another thread mid-load is not evicted.

Dependency recording used to diff sys.modules around the rxconfig import,
so a concurrent import from another thread was misattributed to rxconfig
and evicted from sys.modules on the next config load.

Args:
tmp_path: The pytest tmp_path fixture.
monkeypatch: The pytest monkeypatch fixture.
"""
import textwrap
import types

(tmp_path / "rxconfig.py").write_text(
textwrap.dedent(
"""
import _config_race_gate
import reflex as rx

_config_race_gate.in_load.set()
_config_race_gate.release.wait(timeout=5)
config = rx.Config(app_name="depapp")
"""
)
)
(tmp_path / "side_module.py").write_text("value = 42\n")
gate = types.ModuleType("_config_race_gate")
gate.in_load = threading.Event() # pyright: ignore[reportAttributeAccessIssue]
gate.release = threading.Event() # pyright: ignore[reportAttributeAccessIssue]
monkeypatch.setitem(sys.modules, "_config_race_gate", gate)
monkeypatch.chdir(tmp_path)
sys.modules.pop("side_module", None)

loader = threading.Thread(target=reflex_base.config._load_config)
loader.start()
try:
assert gate.in_load.wait(timeout=5)
# Import a project-local module from this thread while rxconfig loads.
import side_module # noqa: F401 # pyright: ignore[reportMissingImports]
finally:
gate.release.set()
loader.join(timeout=5)
assert not loader.is_alive()

assert "side_module" not in reflex_base.config._config_module_deps
# A second load must not evict the concurrently imported module.
gate.release.set()
reflex_base.config._load_config()
assert "side_module" in sys.modules
sys.modules.pop("side_module", None)
sys.modules.pop("rxconfig", None)
reflex_base.config._config_module_deps.discard("rxconfig")


def test_load_config_survives_rxconfig_rebuilding_meta_path(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
):
"""A load succeeds even if rxconfig.py rebuilds sys.meta_path.

Cleanup used to call sys.meta_path.remove(recorder) unconditionally in a
finally block, so an rxconfig that rebound sys.meta_path turned a
successful load into a ValueError (or masked the real import error). The
recorder is now permanent and reinstalled on the next load.

Args:
tmp_path: The pytest tmp_path fixture.
monkeypatch: The pytest monkeypatch fixture.
"""
import textwrap

(tmp_path / "rxconfig.py").write_text(
textwrap.dedent(
"""
import sys
import reflex as rx
from reflex_base.config import _import_recorder

sys.meta_path = [f for f in sys.meta_path if f is not _import_recorder]
config = rx.Config(app_name="metapathapp")
"""
)
)
monkeypatch.chdir(tmp_path)
try:
config = reflex_base.config._load_config()
assert config.app_name == "metapathapp"
assert reflex_base.config._import_recorder not in sys.meta_path
# The next load reinstalls the recorder, so deps are recorded again.
reflex_base.config._load_config()
assert "rxconfig" in reflex_base.config._config_module_deps
finally:
sys.modules.pop("rxconfig", None)
reflex_base.config._config_module_deps.discard("rxconfig")
if reflex_base.config._import_recorder not in sys.meta_path:
sys.meta_path.insert(0, reflex_base.config._import_recorder)
Loading