Skip to content

Claim the Document thread before running app code - #8693

Merged
philippjfr merged 1 commit into
holoviz:mainfrom
SimonHeybrock:8691-hold-thread-id
Aug 1, 2026
Merged

Claim the Document thread before running app code#8693
philippjfr merged 1 commit into
holoviz:mainfrom
SimonHeybrock:8691-hold-thread-id

Conversation

@SimonHeybrock

@SimonHeybrock SimonHeybrock commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

state._thread_id is only recorded in init_doc(), which runs after the app callable returns. While the app callable is running it is therefore None, and hold() decides whether it was called off the Document's thread with threaded = state._current_thread != state._thread_id — so it unconditionally concludes it is off-thread. Rather than unholding on exit it schedules _unhold as a next-tick callback and restores _hold, leaving the Document held for the rest of the build. Bokeh then constructs the ServerSession, whose __init__ registers every session callback already on the Document, and the deferred unhold afterwards replays the queued SessionCallbackAdded event — registering the same callback a second time.

Every session opened by an app that registers a session callback after a hold() block therefore logs ValueError: A callback of the same type has already been added with this ID:

import panel as pn

def app():
    with pn.io.hold():
        pass
    pn.state.add_periodic_callback(lambda: None, period=1000)
    return pn.pane.Markdown("hello")

pn.serve(app, port=5051, show=False)

Beyond the log noise, DocumentCallbackManager.unhold() dispatches in a plain loop, so the exception drops every event still queued behind the offending one. In the app where I hit this that was 38 of the 39 events queued during the initial render.

The Document's thread is knowable before user code runs, so Application.initialize_document now claims it. That writes the same value init_doc writes moments later — init_doc runs synchronously inside the same build — just early enough for the app callable to see it. init_doc keeps its own assignment, which still covers Panel objects embedded in a plain Bokeh Application.

The alternative I originally suggested on the issue, guarding the comparison with state._thread_id is not None and ..., resolves the ambiguity by guessing that an unset thread id means "on-thread". That guess also fires where the thread id is legitimately never set — notably notebooks, where init_doc returns early because there is no session_context — and would silently move hold() there onto a different path, so I went with making the thread id known instead.

Fixes #8691

A question on #8619

This moves a build-time hold() from the threaded branch of the finally chain onto the not state._connected branch, which drops the hold policy but leaves _held_events populated for an unrelated later unhold() to flush. Build-time holds behaved exactly that way before #8619 reordered the chain, so this restores the older behaviour rather than introducing something new — but was that reordering meant to change what happens to events queued while the app is still being built? Raised separately in #8692, since the leftover events outlive this PR either way.

AI Disclosure

Tool & Model: Claude Code + Opus 5

Usage: Claude Code diagnosed the root cause against the Panel and Bokeh sources, chose where the fix should live, wrote the fix and the regression test, and ran the verification — the reproducer above before and after, the regression test confirmed failing on the parent commit, and the full non-UI test suite compared against a baseline run (no new failures; the 18 pre-existing ones are missing JS bundles and bokeh_fastapi in my environment). I reported the original issue and reviewed the change.

  • I have tested all AI-generated content in my PR.
  • I take responsibility for all AI-generated content in my PR.

Checklist

  • Tests added and are passing

state._thread_id was only recorded in init_doc(), which runs after the
app callable returns. During the build it was therefore None, so hold()
concluded it was running off the Document's thread and, instead of
unholding on exit, deferred the unhold to a next-tick callback. The
Document stayed held for the rest of the build, and the deferred unhold
then replayed the queued events after Bokeh had already constructed the
ServerSession -- registering every session callback added after the
hold a second time and raising "A callback of the same type has already
been added with this ID".

Recording the thread in Application.initialize_document sets the same
value init_doc would, just early enough for user code to see it.

Fixes holoviz#8691
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.04%. Comparing base (c97e809) to head (59a26ba).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8693      +/-   ##
==========================================
- Coverage   86.15%   86.04%   -0.11%     
==========================================
  Files         348      348              
  Lines       57156    57171      +15     
==========================================
- Hits        49242    49195      -47     
- Misses       7914     7976      +62     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SimonHeybrock

Copy link
Copy Markdown
Contributor Author

@philippjfr I am not sure why the CI run failed, could you have a look?

@philippjfr

Copy link
Copy Markdown
Member

Thanks @SimonHeybrock, test failure was just flaky, merging.

@philippjfr
philippjfr merged commit f1c0bb4 into holoviz:main Aug 1, 2026
46 of 51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pn.io.hold() in the app callable leaks the hold, causing duplicate session-callback registration

2 participants