Skip to content

Commit

Permalink
Bug fix for corrupted hook state (#1254)
Browse files Browse the repository at this point in the history
* Bug fix for corrupted hook state - change hook state to a contextvar

---------

Co-authored-by: James Hutchison <122519877+JamesHutchison@users.noreply.github.com>
  • Loading branch information
Archmonger and JamesHutchison authored Feb 11, 2025
1 parent d5a897e commit e5e2661
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 57 deletions.
1 change: 1 addition & 0 deletions docs/source/about/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
------
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ commands = [
]
artifacts = []


#############################
# >>> Hatch Test Runner <<< #
#############################
Expand Down
60 changes: 43 additions & 17 deletions src/reactpy/core/_life_cycle_hook.py
Original file line number Diff line number Diff line change
@@ -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")

Expand All @@ -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:
Expand All @@ -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`
Expand All @@ -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: ...
Expand All @@ -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()
Expand Down Expand Up @@ -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] = []
Expand All @@ -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]
Expand Down Expand Up @@ -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
6 changes: 4 additions & 2 deletions src/reactpy/core/_thread_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions src/reactpy/core/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand Down
27 changes: 16 additions & 11 deletions src/reactpy/core/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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(
Expand Down
25 changes: 14 additions & 11 deletions src/reactpy/pyscript/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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"
Expand All @@ -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}"
Expand Down
4 changes: 2 additions & 2 deletions src/reactpy/testing/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions src/reactpy/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 17 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit e5e2661

Please sign in to comment.