fix(openviking): join runtime-autostart thread on shutdown (SIGABRT-at-exit)

[koshaji]

Problem

OpenVikingMemoryProvider.shutdown() joins its in-flight writers, deferred-commit threads, and prefetch threads — but not _runtime_start_thread, the tracked daemon=True waiter started by _start_runtime_openviking_waiter(). That thread runs _finish_runtime_openviking_start(), which blocks on network I/O: _wait_for_openviking_health() polls reachability for up to _LOCAL_OPENVIKING_AUTOSTART_TIMEOUT (60s) and then issues a _VikingClient.health() request.

If the local OpenViking runtime is slow or unreachable, that waiter can still be blocked in network I/O at interpreter exit. CPython then forcibly kills it during Py_FinalizeEx (PyThread_exit_thread → __pthread_unwind → abort()), producing SIGABRT (exit 134) with no traceback.

This is the same daemon-thread-at-exit failure class that was fixed for the Honcho provider (gh-97940 / bpo-20526) — OpenViking has the same shape but one tracked thread was omitted from shutdown().

Fix

1. shutdown() now joins _runtime_start_thread (timeout-bounded) alongside the other tracked threads.
2. _wait_for_openviking_health() gains an optional should_stop callback; the waiter passes lambda: self._shutting_down so the poll loop bails out promptly once shutdown() sets the flag — otherwise the join would just time out against the 60s autostart wait and leave the thread alive (defeating the purpose). This mirrors how the Honcho fix closes the httpx client to unblock its worker so the subsequent join lands.

Tests

tests/plugins/memory/test_openviking_shutdown.py:

• the health waiter short-circuits (no network probe) when should_stop returns true;
• shutdown() actually waits for the runtime-start thread (the fake waiter's post-stop work has completed by the time shutdown() returns; without the join it would not).

Both pass locally. Found while hardening memory-provider teardown after the Honcho oneshot SIGABRT fix (#49498).

[Copilot]

Pull request overview

Fixes a daemon-thread-at-interpreter-exit crash class in the OpenViking memory provider by ensuring the runtime-autostart waiter thread can be stopped promptly and is joined during shutdown.

Changes:

• Add a should_stop callback to _wait_for_openviking_health() and pass the provider shutdown flag from the runtime-autostart waiter.
• Update OpenVikingMemoryProvider.shutdown() to also join the runtime-autostart thread.
• Add tests covering the new early-exit behavior and verifying shutdown() waits for the runtime-start thread to complete.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
plugins/memory/openviking/__init__.py	Adds shutdown-aware health waiting and joins the autostart waiter thread during provider teardown.
tests/plugins/memory/test_openviking_shutdown.py	New tests to prevent regressions around shutdown and daemon waiter behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[alt-glitch]

Related: #49498 (Honcho memory-provider SIGABRT-at-exit fix).

Same daemon-thread-at-exit failure class, different provider: this joins OpenViking's tracked _runtime_start_thread (the autostart health waiter) in shutdown() — the one thread the existing teardown omitted — and adds a should_stop short-circuit so the join lands instead of timing out against the 60s autostart wait. Sibling fix to #49498, not a duplicate.

[teknium1]

Thanks for identifying the unjoined runtime-autostart thread. The underlying gap is present on current main: plugins/memory/openviking/__init__.py:2042 starts _runtime_start_thread as a daemon, and shutdown() at :3320-3341 does not join it.

Problems

• plugins/memory/openviking/__init__.py:3349 adds self._prefetch_lock / self._prefetch_threads, but neither attribute exists in the provider. The added block makes every shutdown() raise AttributeError.
• The new should_stop kwarg at plugins/memory/openviking/__init__.py:2069 invalidates the exact kwargs assertion in tests/plugins/memory/test_openviking_provider.py:915-918; this existing test must be updated.

Suggested changes

• Drop the unrelated prefetch block unless matching tracked prefetch state is added in this provider.
• Extend the existing runtime-waiter test to verify the callback is supplied and returns false before shutdown.

Automated hermes-sweeper review.

[teknium1]

OpenVikingMemoryProvider has no _prefetch_lock or _prefetch_threads field (verified across the current provider), so this makes every shutdown() raise AttributeError. Please remove this unrelated block, or add the actual tracked prefetch state and its producer if prefetch is intended here.

[teknium1]

This new keyword breaks the existing exact-kwargs assertion in tests/plugins/memory/test_openviking_provider.py:915-918, which currently expects only timeout_seconds. Update that test to expect and exercise the callback.

[kshitijk4poor]

Superseded by #51952, which includes your commit cherry-picked with authorship preserved (joining the runtime-autostart thread on shutdown to fix the SIGABRT-at-exit, adapted to keep the existing memory-write drain). Thanks @koshaji — credit retained in git history and scripts/release.py.