Skip to content

Catalog UI shutdown must not auto-install the ui extension to call stop_ui_server() - #347

Closed
rakesh0x wants to merge 1 commit into
Hebbian-Robotics:mainfrom
rakesh0x:fix/catalog-ui-stop-ui-server
Closed

rakesh0x wants to merge 1 commit into
Hebbian-Robotics:mainfrom
rakesh0x:fix/catalog-ui-stop-ui-server

Conversation

@rakesh0x

@rakesh0x rakesh0x commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Closes #336

What and why

serve_catalog_ui sets server_started = True as soon as _start_duckdb_ui_server returns, and shutdown calls stop_ui_server() on that flag alone. stop_ui_server() is a function the ui extension provides — calling it on a connection that never loaded the extension makes DuckDB auto-install the extension from the network at shutdown, to run what is a no-op: there is no server to stop if the extension was never loaded.

I measured the cost with a cold extension directory (extension_directory pointed at an empty dir):

before call: ('ui', False, False)
stop_ui_server ok
elapsed: 3.16s
after call: ('ui', True, True)

That network fetch inside a teardown that another thread waits on with a fixed timeout is almost certainly the flaky test_catalog_ui timing failure three contributors reported on 2026-08-31 (#326, #327, #328) — tests/test_catalog_ui.py stubs _start_duckdb_ui_server, so the extension never loads, but server_started is still True and teardown calls stop_ui_server() anyway.

The fix

Guard the call on the extension actually being loaded on that connection:

def _ui_extension_is_loaded(catalog_connection: duckdb.DuckDBPyConnection) -> bool:
    row = catalog_connection.execute(
        "SELECT count(*) FROM duckdb_extensions() WHERE extension_name = 'ui' AND loaded"
    ).fetchone()
    return row is not None and int(row[0]) > 0

and gate stop_ui_server() on server_started and _ui_extension_is_loaded(...). Production is unchanged: the real starter runs INSTALL ui / LOAD ui before the server starts, so the loaded check passes and the stop call still happens. A connection that never loaded the extension now closes directly.

DoD coverage

  1. Shutdown does not call stop_ui_server() when ui is not loaded — test_catalog_ui_shutdown_does_not_stop_a_server_the_ui_extension_never_loaded pins it by recording every statement executed on the connection (per the issue: recording statements is enough; DuckDB's install behavior is not the point).
  2. A session whose ui extension is loaded still stops its server — test_catalog_ui_shutdown_stops_the_server_when_the_ui_extension_is_loaded pins the stop call on the loaded branch, and that the connection still closes.
  3. Existing catalog UI startup, port-conflict, and refresh behavior is untouched — the full existing suite passes unchanged.

Validation

uv sync --locked
uv run ruff check        # All checks passed!
uv run ruff format       # (check) clean
uv run ty check          # only the pre-existing cv2 optional-import diagnostic
uv run pytest -q tests/test_catalog_ui.py   # 22 passed

Found by me while working on #291; the reachability trace (stubbed starter → server_started = True → network fetch in teardown) is from the issue.

_stop_duckdb_ui_server called stop_ui_server() whenever the server start
hook ran, but the function only exists once the ui extension is loaded.
When it is not (e.g. the lifecycle test monkeypatches the start hook),
DuckDB auto-installs the extension from the network during shutdown --
seconds on a slow connection, and pointless: there is no server to stop
if the extension was never loaded. That network fetch inside a teardown
another thread waits on with a fixed timeout is almost certainly the
flaky test_catalog_ui timing failure reported on Hebbian-Robotics#326, Hebbian-Robotics#327 and Hebbian-Robotics#328.

Guard the call with a duckdb_extensions() loaded check: production
always LOADs ui before the server starts, so the stop call is preserved,
and a connection that never loaded the extension now closes directly.

Pin both shutdown branches with tests that record the statements
executed on the connection (Hebbian-Robotics#336 DoD): the stubbed-starter path must
issue no stop_ui_server() call, and a session whose ui extension is
loaded must still stop its server.

Closes Hebbian-Robotics#336
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

👋 Hi @rakesh0x — thanks for the contribution! To keep starter issues available
for other contributors and give every pull request a real review, we accept
1 open pull request per contributor at a time.

You already have #334 open, so this one is being closed automatically.
Once your open pull request is merged or closed, feel free to reopen this one —
no work is lost.

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.

Catalog UI shutdown installs the ui extension from the network to call stop_ui_server()

1 participant