Sitelet https://github.com/PECOS-packages/PECOS/pull/1064
Skip to content

Stop a QisEngine's dynamic worker on drop with a bounded join - #1064

Open
ciaranra wants to merge 2 commits into
devfrom
fix/engine-drop-joins-worker
Open

ciaranra wants to merge 2 commits into
devfrom
fix/engine-drop-joins-worker

Conversation

@ciaranra

@ciaranra ciaranra commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Fixes #1059. Builds on #1057 (merged): reset cancellation and the abort export it reuses.

Problem

QisEngine runs a user program on a persistent dynamic worker thread. Neither QisEngine nor PersistentDynamicWorker implemented Drop. Dropping the engine therefore detached the thread, and a program in the middle of a shot kept running after the engine was gone. The worker's field comment said the handle was "joined on drop", which was not true.

Change

  • QisEngine::drop requests cancellation of a running shot through the shot's sync handle: the Cancel a running dynamic shot on reset and reclaim the worker-owned interface #1057 abort export, aimed at the engine's own execution context. It does nothing else. It performs no runtime or interface reset and destroys no execution context, so the deliberate shutdown mitigations in SeleneRuntime::Drop and QisHeliosInterface::Drop are unchanged. A failed abort is logged.
  • PersistentDynamicWorker::drop owns the thread's lifecycle:
    • It closes the work channel and keeps the result receiver alive, so the worker's final send cannot fail.
    • It waits for the thread for up to 10 s (the reset deadline), then joins it. A worker panic is logged with its message.
    • If the worker misses the deadline, it logs a warning, naming an abort failure if there was one, and detaches the thread. A destructor that hangs is worse than one detached thread.
  • The bound covers only the thread wait. The abort itself takes the context's sync lock, which is only ever held briefly.
  • The module docs and field comments now describe this behaviour and its limit.

Tests

Each test was checked against a targeted mutation that removes the behaviour it guards. The witness for "the thread exited" is a test-only counter that a guard decrements at the very end of the worker thread. Counts are per engine, shared through Clone, so tests running concurrently in one binary stay isolated.

  • Real Helios worker:
    • A worker blocked waiting for a measurement is cancelled and joined when the engine drops, including when the drop happens on another thread.
    • An idle worker after a completed shot is joined.
    • A queued result that was never consumed does not prevent the join.
  • Fixture workers:
    • A worker that does not finish: drop returns within the bound and warns. This test fails, rather than hangs, if the bound is removed.
    • A failed abort: the channel still closes, and the warning names the abort failure.
    • A worker that panicked: drop does not panic, and the warning carries the panic message.
    • A completed shot is not aborted.
    • Drop never calls a runtime or interface reset.
  • MonteCarlo with several workers: repeated runs, programs that fail, and a quantum engine that fails mid-shot while the worker is blocked on a read. All workers are joined in every case.

Verification

Run locally on Linux:

  • cargo test --locked -p pecos-qis (with and without --features selene-runtimes) and -p pecos-engines, with RUST_BACKTRACE=1
  • cargo clippy --workspace --all-targets --all-features -- -D warnings, cargo fmt --all -- --check, and pre-commit on the changed files
  • just build-debug, just pytest-ci-core and just rslib-rust-test on the first commit; the follow-up commit changes only tests and a log message.

@ciaranra ciaranra added the ci:full-rust Run full Rust tests on Linux, macOS, and Windows before merge label Oct 6, 2026
@ciaranra
ciaranra force-pushed the fix/midshot-reset-reclaims-interface branch from 7dfeb35 to 97e5802 Compare October 6, 2026 23:09
@ciaranra
ciaranra force-pushed the fix/engine-drop-joins-worker branch from 1941880 to f62d543 Compare October 6, 2026 23:09
@ciaranra
ciaranra force-pushed the fix/engine-drop-joins-worker branch from f62d543 to 88d5a60 Compare October 7, 2026 01:40
@ciaranra
ciaranra force-pushed the fix/midshot-reset-reclaims-interface branch from 97578fb to ac6ea44 Compare October 7, 2026 03:34
@ciaranra
ciaranra force-pushed the fix/engine-drop-joins-worker branch from 88d5a60 to 6aea8d9 Compare October 7, 2026 03:34
@ciaranra
ciaranra force-pushed the fix/midshot-reset-reclaims-interface branch from ac6ea44 to 8c2c116 Compare October 7, 2026 05:15
@ciaranra
ciaranra force-pushed the fix/engine-drop-joins-worker branch from 6aea8d9 to f29eda9 Compare October 7, 2026 05:15
Base automatically changed from fix/midshot-reset-reclaims-interface to dev October 7, 2026 19:58
@ciaranra
ciaranra force-pushed the fix/engine-drop-joins-worker branch from f29eda9 to d9d10d3 Compare October 7, 2026 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:full-rust Run full Rust tests on Linux, macOS, and Windows before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

QisEngine never joins its dynamic worker thread; a running program outlives the engine

1 participant