holoviz/panel · #9008

fix: Only treat the document lock as held by the context that acquired it

philippjfr · merged Nov 2, 20264 files · 107 + / 4 −
panel/io/server.py27 + / 1 −
@@ -38,6 +38,9 @@ from bokeh.embed.wrappers import wrap_in_script_tag from bokeh.server.contexts import _RequestProxy as BkRequestProxy from bokeh.server.server import Server as BokehServer+from bokeh.server.session import (+    ServerSession, _needs_document_lock, _needs_document_lock_on_loop,+) from bokeh.server.urls import per_app_patterns, toplevel_patterns from bokeh.server.views.autoload_js_handler import (     AutoloadJsHandler as BkAutoloadJsHandler,@@ -104,7 +107,6 @@     from bokeh.core.types import ID     from bokeh.document.document import DocJson     from bokeh.embed.bundle import Bundle-    from bokeh.server.session import ServerSession     from jinja2 import Template      from .application import TViewableFuncOrPath@@ -734,6 +736,30 @@ def validate_static_dirs(static_dirs: Mapping[str, str]) -> dict[str, str]:     return static  +def _track_document_lock(func):+    if inspect.iscoroutinefunction(func):+        @wraps(func)+        async def wrapper(self, *args, **kwargs):+            with state._own_document_lock(self):+                return await func(self, *args, **kwargs)+    else:+        @wraps(func)+        def wrapper(self, *args, **kwargs):+            with state._own_document_lock(self):+                return func(self, *args, **kwargs)+    return wrapper++# Record which context holds the document lock so state._unblocked can+# tell the holder apart from other threads while the lock is taken.+ServerSession.with_document_locked = _needs_document_lock(  # type: ignore[method-assign]+    _track_document_lock(ServerSession.with_document_locked.__wrapped__)  # type: ignore[attr-defined]+)+for _method in ('_handle_push', '_handle_patch'):+    setattr(ServerSession, _method, _needs_document_lock_on_loop(+        _track_document_lock(getattr(ServerSession, _method).__wrapped__)+    ))++ # Patch Server to attach task factory to asyncio loop and handle Admin server context class Server(BokehServer): 
panel/io/state.py28 + / 2 −
@@ -157,6 +157,12 @@ class _state(param.Parameterized):     # Holds temporary curdoc overrides per thread     _curdoc: ContextVar[Document | None] = ContextVar('curdoc', default=None) +    # A token identifying the document lock acquisition the current+    # context runs under, matched against the token of the acquisition+    # each session is currently held by+    _document_lock: ContextVar[object | None] = ContextVar('document_lock', default=None)+    _document_lock_owners: t.ClassVar[WeakKeyDictionary[ServerSession, object]] = WeakKeyDictionary()+     # Whether to hold comm events     _hold: bool = False @@ -341,10 +347,30 @@ def _document_lock_held(self) -> bool:         is held for the duration. Mutating Bokeh models is safe in that         case since no other code can be touching them concurrently, but         raw socket writes are NOT (see `_on_loop_thread`).++        Only the context the lock was acquired for counts. Another+        callback holding the lock does not make writes from this+        thread safe, so ownership is tracked by `_own_document_lock`.         """         session = getattr(getattr(self.curdoc, 'session_context', None), 'session', None)-        session_lock = getattr(session, '_lock', None)-        return bool(session_lock is not None and session_lock.locked())+        owner = self._document_lock.get()+        return owner is not None and session is not None and self._document_lock_owners.get(session) is owner++    @contextmanager+    def _own_document_lock(self, session: ServerSession) -> Iterator[None]:+        """+        Marks the current context as running under the session's+        document lock. Must only be entered while the lock is held.+        """+        owner = object()+        self._document_lock_owners[session] = owner+        token = self._document_lock.set(owner)+        try:+            yield+        finally:+            self._document_lock.reset(token)+            if self._document_lock_owners.get(session) is owner:+                del self._document_lock_owners[session]      def _install_thread_pool(self, loop: asyncio.AbstractEventLoop | None = None) -> None:         """
panel/tests/io/test_document.py51 + / 0 −
@@ -380,6 +380,57 @@ async def trigger():     assert 'on_loop' in seen  +@pytest.mark.xdist_group(name="server")+def test_unblocked_only_for_document_lock_holder():+    """+    While a locked callback holds the document lock on a worker thread,+    only that thread may write to the Document directly. Any other+    thread has to schedule its changes, otherwise Bokeh raises because+    the writes race with the holder releasing the lock.+    """+    slider = IntSlider()++    serve_and_request(slider)+    wait_until(lambda: bool(slider._documents))++    doc = list(slider._documents)[0]+    session = doc.session_context.session+    asyncio_loop = doc.session_context.server_context.application_context.io_loop.asyncio_loop+    state._connected[doc] = True++    entered, release = threading.Event(), threading.Event()+    seen = {}++    def holder():+        with set_curdoc(doc):+            seen['holder'] = state._unblocked(doc)+        entered.set()+        release.wait(timeout=5)++    def other():+        with set_curdoc(doc):+            seen['other'] = state._unblocked(doc)++    async def trigger():+        await session.with_document_locked(holder)++    future = asyncio.run_coroutine_threadsafe(trigger(), asyncio_loop)+    try:+        assert entered.wait(timeout=5)+        thread = threading.Thread(target=other)+        thread.start()+        thread.join()+    finally:+        release.set()+    future.result(timeout=5)++    assert seen == {'holder': True, 'other': False}++    # Once released the holder's context no longer counts as owning the lock+    with set_curdoc(doc):+        assert not state._document_lock_held++ @pytest.mark.xdist_group(name="server") def test_threaded_hold_does_not_boomerang_python_changes():     """
panel/tests/ui/template/test_fastlisttemplate.py1 + / 1 −
@@ -114,7 +114,7 @@ def test_fast_list_template_both_sizing_modes_fill_height(page, sizing_mode):     card = page.locator(f"#main .card-margin.{sizing_mode}")     expect(card).to_have_count(1)     wait_until(lambda: card.bounding_box()['height'] > 300, page)-    +  ACTIVE_TOOL_COLORS = """ () => {