Bug fix for corrupted hook state (#1254)

* Bug fix for corrupted hook state - change hook state to a contextvar

---------

Co-authored-by: James Hutchison <[email protected]>

Author: Archmonger

docs/source/about/changelog.rst

@@ -56,6 +56,7 @@ Unreleased
 **Fixed**
 
 - :pull:`1239` - Fixed a bug where script elements would not render to the DOM as plain text.
+- :pull:`1254` - Fixed a bug where ``RuntimeError("Hook stack is in an invalid state")`` errors would be provided when using a webserver that reuses threads.
 
 v1.1.0
 ------

pyproject.toml

@@ -80,7 +80,6 @@ commands = [
 ]
 artifacts = []
 
-
 #############################
 # >>> Hatch Test Runner <<< #
 #############################

src/reactpy/core/_life_cycle_hook.py

@@ -1,13 +1,16 @@
 from __future__ import annotations
 
 import logging
+import sys
 from asyncio import Event, Task, create_task, gather
+from contextvars import ContextVar, Token
 from typing import Any, Callable, Protocol, TypeVar
 
 from anyio import Semaphore
 
 from reactpy.core._thread_local import ThreadLocal
 from reactpy.types import ComponentType, Context, ContextProviderType
+from reactpy.utils import Singleton
 
 T = TypeVar("T")
 
@@ -18,16 +21,39 @@ async def __call__(self, stop: Event) -> None: ...
 
 logger = logging.getLogger(__name__)
 
-_HOOK_STATE: ThreadLocal[list[LifeCycleHook]] = ThreadLocal(list)
 
+class _HookStack(Singleton):  # pragma: no cover
+    """A singleton object which manages the current component tree's hooks.
+    Life cycle hooks can be stored in a thread local or context variable depending
+    on the platform."""
 
-def current_hook() -> LifeCycleHook:
-    """Get the current :class:`LifeCycleHook`"""
-    hook_stack = _HOOK_STATE.get()
-    if not hook_stack:
-        msg = "No life cycle hook is active. Are you rendering in a layout?"
-        raise RuntimeError(msg)
-    return hook_stack[-1]
+    _state: ThreadLocal[list[LifeCycleHook]] | ContextVar[list[LifeCycleHook]] = (
+        ThreadLocal(list) if sys.platform == "emscripten" else ContextVar("hook_state")
+    )
+
+    def get(self) -> list[LifeCycleHook]:
+        return self._state.get()
+
+    def initialize(self) -> Token[list[LifeCycleHook]] | None:
+        return None if isinstance(self._state, ThreadLocal) else self._state.set([])
+
+    def reset(self, token: Token[list[LifeCycleHook]] | None) -> None:
+        if isinstance(self._state, ThreadLocal):
+            self._state.get().clear()
+        elif token:
+            self._state.reset(token)
+        else:
+            raise RuntimeError("Hook stack is an ContextVar but no token was provided")
+
+    def current_hook(self) -> LifeCycleHook:
+        hook_stack = self.get()
+        if not hook_stack:
+            msg = "No life cycle hook is active. Are you rendering in a layout?"
+            raise RuntimeError(msg)
+        return hook_stack[-1]
+
+
+HOOK_STACK = _HookStack()
 
 
 class LifeCycleHook:
@@ -37,7 +63,7 @@ class LifeCycleHook:
     a component is first rendered until it is removed from the layout. The life cycle
     is ultimately driven by the layout itself, but components can "hook" into those
     events to perform actions. Components gain access to their own life cycle hook
-    by calling :func:`current_hook`. They can then perform actions such as:
+    by calling :func:`HOOK_STACK.current_hook`. They can then perform actions such as:
 
     1. Adding state via :meth:`use_state`
     2. Adding effects via :meth:`add_effect`
@@ -57,7 +83,7 @@ class LifeCycleHook:
         .. testcode::
 
             from reactpy.core._life_cycle_hook import LifeCycleHook
-            from reactpy.core.hooks import current_hook
+            from reactpy.core.hooks import HOOK_STACK
 
             # this function will come from a layout implementation
             schedule_render = lambda: ...
@@ -75,15 +101,15 @@ class LifeCycleHook:
                 ...
 
                 # the component may access the current hook
-                assert current_hook() is hook
+                assert HOOK_STACK.current_hook() is hook
 
                 # and save state or add effects
-                current_hook().use_state(lambda: ...)
+                HOOK_STACK.current_hook().use_state(lambda: ...)
 
                 async def my_effect(stop_event):
                     ...
 
-                current_hook().add_effect(my_effect)
+                HOOK_STACK.current_hook().add_effect(my_effect)
             finally:
                 await hook.affect_component_did_render()
 
@@ -130,7 +156,7 @@ def __init__(
         self._scheduled_render = False
         self._rendered_atleast_once = False
         self._current_state_index = 0
-        self._state: tuple[Any, ...] = ()
+        self._state: list = []
         self._effect_funcs: list[EffectFunc] = []
         self._effect_tasks: list[Task[None]] = []
         self._effect_stops: list[Event] = []
@@ -157,7 +183,7 @@ def use_state(self, function: Callable[[], T]) -> T:
         if not self._rendered_atleast_once:
             # since we're not initialized yet we're just appending state
             result = function()
-            self._state += (result,)
+            self._state.append(result)
         else:
             # once finalized we iterate over each succesively used piece of state
             result = self._state[self._current_state_index]
@@ -232,13 +258,13 @@ def set_current(self) -> None:
         This method is called by a layout before entering the render method
         of this hook's associated component.
         """
-        hook_stack = _HOOK_STATE.get()
+        hook_stack = HOOK_STACK.get()
         if hook_stack:
             parent = hook_stack[-1]
             self._context_providers.update(parent._context_providers)
         hook_stack.append(self)
 
     def unset_current(self) -> None:
         """Unset this hook as the active hook in this thread"""
-        if _HOOK_STATE.get().pop() is not self:
+        if HOOK_STACK.get().pop() is not self:
             raise RuntimeError("Hook stack is in an invalid state")  # nocov

src/reactpy/core/_thread_local.py

@@ -5,8 +5,10 @@
 _StateType = TypeVar("_StateType")
 
 
-class ThreadLocal(Generic[_StateType]):
-    """Utility for managing per-thread state information"""
+class ThreadLocal(Generic[_StateType]):  # pragma: no cover
+    """Utility for managing per-thread state information. This is only used in
+    environments where ContextVars are not available, such as the `pyodide`
+    executor."""
 
     def __init__(self, default: Callable[[], _StateType]):
         self._default = default

src/reactpy/core/hooks.py

@@ -19,7 +19,7 @@
 from typing_extensions import TypeAlias
 
 from reactpy.config import REACTPY_DEBUG
-from reactpy.core._life_cycle_hook import current_hook
+from reactpy.core._life_cycle_hook import HOOK_STACK
 from reactpy.types import Connection, Context, Key, Location, State, VdomDict
 from reactpy.utils import Ref
 
@@ -83,7 +83,7 @@ def __init__(
         else:
             self.value = initial_value
 
-        hook = current_hook()
+        hook = HOOK_STACK.current_hook()
 
         def dispatch(new: _Type | Callable[[_Type], _Type]) -> None:
             next_value = new(self.value) if callable(new) else new  # type: ignore
@@ -139,7 +139,7 @@ def use_effect(
     Returns:
         If not function is provided, a decorator. Otherwise ``None``.
     """
-    hook = current_hook()
+    hook = HOOK_STACK.current_hook()
     dependencies = _try_to_infer_closure_values(function, dependencies)
     memoize = use_memo(dependencies=dependencies)
     cleanup_func: Ref[_EffectCleanFunc | None] = use_ref(None)
@@ -212,7 +212,7 @@ def use_async_effect(
     Returns:
         If not function is provided, a decorator. Otherwise ``None``.
     """
-    hook = current_hook()
+    hook = HOOK_STACK.current_hook()
     dependencies = _try_to_infer_closure_values(function, dependencies)
     memoize = use_memo(dependencies=dependencies)
     cleanup_func: Ref[_EffectCleanFunc | None] = use_ref(None)
@@ -280,7 +280,7 @@ def use_debug_value(
 
     if REACTPY_DEBUG.current and old.current != new:
         old.current = new
-        logger.debug(f"{current_hook().component} {new}")
+        logger.debug(f"{HOOK_STACK.current_hook().component} {new}")
 
 
 def create_context(default_value: _Type) -> Context[_Type]:
@@ -308,7 +308,7 @@ def use_context(context: Context[_Type]) -> _Type:
 
     See the full :ref:`Use Context` docs for more information.
     """
-    hook = current_hook()
+    hook = HOOK_STACK.current_hook()
     provider = hook.get_context_provider(context)
 
     if provider is None:
@@ -361,7 +361,7 @@ def __init__(
         self.value = value
 
     def render(self) -> VdomDict:
-        current_hook().set_context_provider(self)
+        HOOK_STACK.current_hook().set_context_provider(self)
         return {"tagName": "", "children": self.children}
 
     def __repr__(self) -> str:
@@ -554,7 +554,7 @@ def use_ref(initial_value: _Type) -> Ref[_Type]:
 
 
 def _use_const(function: Callable[[], _Type]) -> _Type:
-    return current_hook().use_state(function)
+    return HOOK_STACK.current_hook().use_state(function)
 
 
 def _try_to_infer_closure_values(

src/reactpy/core/serve.py

@@ -9,6 +9,7 @@
 from anyio.abc import TaskGroup
 
 from reactpy.config import REACTPY_DEBUG
+from reactpy.core._life_cycle_hook import HOOK_STACK
 from reactpy.types import LayoutEventMessage, LayoutType, LayoutUpdateMessage
 
 logger = getLogger(__name__)
@@ -63,18 +64,22 @@ async def _single_outgoing_loop(
     send: SendCoroutine,
 ) -> None:
     while True:
-        update = await layout.render()
+        token = HOOK_STACK.initialize()
         try:
-            await send(update)
-        except Exception:  # nocov
-            if not REACTPY_DEBUG.current:
-                msg = (
-                    "Failed to send update. More info may be available "
-                    "if you enabling debug mode by setting "
-                    "`reactpy.config.REACTPY_DEBUG.current = True`."
-                )
-                logger.error(msg)
-            raise
+            update = await layout.render()
+            try:
+                await send(update)
+            except Exception:  # nocov
+                if not REACTPY_DEBUG.current:
+                    msg = (
+                        "Failed to send update. More info may be available "
+                        "if you enabling debug mode by setting "
+                        "`reactpy.config.REACTPY_DEBUG.current = True`."
+                    )
+                    logger.error(msg)
+                raise
+        finally:
+            HOOK_STACK.reset(token)
 
 
 async def _single_incoming_loop(

src/reactpy/pyscript/utils.py

@@ -145,6 +145,8 @@ def extend_pyscript_config(
 
 
 def reactpy_version_string() -> str:  # pragma: no cover
+    from reactpy.testing.common import GITHUB_ACTIONS
+
     local_version = reactpy.__version__
 
     # Get a list of all versions via `pip index versions`
@@ -170,14 +172,16 @@ def reactpy_version_string() -> str:  # pragma: no cover
             symbol_postion = line.index(latest_version_symbol)
             latest_version = line[symbol_postion + len(latest_version_symbol) :].strip()
 
-    # Return early if local version of ReactPy is available on PyPi
-    if local_version in known_versions:
+    # Return early if the version is available on PyPi and we're not in a CI environment
+    if local_version in known_versions and not GITHUB_ACTIONS:
         return f"reactpy=={local_version}"
 
-    # Begin determining an alternative method of installing ReactPy
-
-    if not latest_version:
-        _logger.warning("Failed to determine the latest version of ReactPy on PyPi. ")
+    # We are now determining an alternative method of installing ReactPy for PyScript
+    if not GITHUB_ACTIONS:
+        _logger.warning(
+            "Your current version of ReactPy isn't available on PyPi. Since a packaged version "
+            "of ReactPy is required for PyScript, we are attempting to find an alternative method..."
+        )
 
     # Build a local wheel for ReactPy, if needed
     dist_dir = Path(reactpy.__file__).parent.parent.parent / "dist"
@@ -202,19 +206,18 @@ def reactpy_version_string() -> str:  # pragma: no cover
             )
             return f"reactpy=={latest_version}"
         _logger.error(
-            "Failed to build a local wheel for ReactPy and could not determine the latest version on PyPi. "
+            "Failed to build a local wheel for ReactPy, and could not determine the latest version on PyPi. "
             "PyScript functionality may not work as expected.",
         )
         return f"reactpy=={local_version}"
 
-    # Move the local file to the web modules directory, if needed
+    # Move the local wheel file to the web modules directory, if needed
     wheel_file = Path(wheel_glob[0])
     new_path = REACTPY_WEB_MODULES_DIR.current / wheel_file.name
     if not new_path.exists():
         _logger.warning(
-            "'reactpy==%s' is not available on PyPi. "
-            "PyScript will utilize a local wheel of ReactPy instead.",
-            local_version,
+            "PyScript will utilize local wheel '%s'.",
+            wheel_file.name,
         )
         shutil.copy(wheel_file, new_path)
     return f"{REACTPY_PATH_PREFIX.current}modules/{wheel_file.name}"

src/reactpy/testing/common.py

@@ -14,7 +14,7 @@
 from typing_extensions import ParamSpec
 
 from reactpy.config import REACTPY_TESTS_DEFAULT_TIMEOUT, REACTPY_WEB_MODULES_DIR
-from reactpy.core._life_cycle_hook import LifeCycleHook, current_hook
+from reactpy.core._life_cycle_hook import HOOK_STACK, LifeCycleHook
 from reactpy.core.events import EventHandler, to_event_handler_function
 
 
@@ -153,7 +153,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
             if self is None:
                 raise RuntimeError("Hook catcher has been garbage collected")
 
-            hook = current_hook()
+            hook = HOOK_STACK.current_hook()
             if self.index_by_kwarg is not None:
                 self.index[kwargs[self.index_by_kwarg]] = hook
             self.latest = hook

src/reactpy/utils.py

@@ -334,3 +334,13 @@ def import_dotted_path(dotted_path: str) -> Any:
     except AttributeError as error:
         msg = f'ReactPy failed to import "{component_name}" from "{module_name}"'
         raise AttributeError(msg) from error
+
+
+class Singleton:
+    """A class that only allows one instance to be created."""
+
+    def __new__(cls, *args, **kw):
+        if not hasattr(cls, "_instance"):
+            orig = super()
+            cls._instance = orig.__new__(cls, *args, **kw)
+        return cls._instance

tests/conftest.py

@@ -44,6 +44,23 @@ def rebuild():
     subprocess.run(["hatch", "build", "-t", "wheel"], check=True)  # noqa: S607, S603
 
 
+@pytest.fixture(autouse=True, scope="function")
+def create_hook_state():
+    """This fixture is a bug fix related to `pytest_asyncio`.
+
+    Usually the hook stack is created automatically within the display fixture, but context
+    variables aren't retained within `pytest_asyncio` async fixtures. As a workaround,
+    this fixture ensures that the hook stack is created before each test is run.
+
+    Ref: https://github.com/pytest-dev/pytest-asyncio/issues/127
+    """
+    from reactpy.core._life_cycle_hook import HOOK_STACK
+
+    token = HOOK_STACK.initialize()
+    yield token
+    HOOK_STACK.reset(token)
+
+
 @pytest.fixture
 async def display(server, page):
     async with DisplayFixture(server, page) as display: