Home

dev / openkara

publicthedavidweng/OpenKara· sync paused
Code Branches Pull requestsIssuesInsights
main
Home Code PRsIssues

#377 ONNX Runtime `in_progress` latch is never cleared when the loader thread panics

closed

Opened by dev · yesterday

devopened this issueAuthor· yesterday

Line references are pinned to main at bac2f765.

Pre-existing on main. Not introduced by #370 — that PR moved this code into separator::activation unchanged, so do not bisect to it.

Where

src-tauri/src/separator/activation.rs L370-397, with the latch declared at L319-325.

What goes wrong

LoadStrategy::commit claims the latch, then releases it from inside the spawned loader thread:

// L370-386
if self.latches.in_progress.swap(true, Ordering::SeqCst) {
    return LoadOutcome::ProcessUnavailable(anyhow::anyhow!(
        "{RUNTIME_LOAD_IN_PROGRESS_MARKER}: ONNX Runtime load is already in progress; restart OpenKara before retrying"
    ));
}

let (sender, receiver) = mpsc::sync_channel(1);
...
thread::spawn(move || {
    let result = loader.load(&runtime_path).map_err(|error| format!("{error:#}"));
    latches.in_progress.store(false, Ordering::SeqCst);
    let _ = sender.send(result);
});

The store(false) at L384 is a plain statement on the thread's happy path. If loader.load panics — anywhere in the load path, including the Windows LoadLibraryExW wrapper — the thread unwinds, the store never executes and the sender is dropped. The caller observes the drop and reports it once:

// L395-397
Err(mpsc::RecvTimeoutError::Disconnected) => LoadOutcome::ArtifactFailed(
    anyhow::anyhow!("ONNX Runtime load watchdog exited before reporting a result"),
),

But in_progress stays true for the life of the process, and it is a process-wide latch in production (PROCESS_LATCHES, L336-339). Every subsequent activation attempt — including one after the user re-downloads or repairs the runtime — short-circuits at L370 with ProcessUnavailable("runtime_parent_load_in_progress: ... restart OpenKara before retrying").

The sticky behavior is correct for the other latch: timed_out is documented as poisoning the process (L319-320) because an abandoned loader may still be inside DllMain. in_progress has no such justification; the design clearly intends it to be released when the attempt finishes, and it is released on both success and a returned error.

Conditions

Any panic inside RuntimeLoader::load. The failure is silent — a panic on a detached thread only prints to stderr — so from the UI the first attempt fails with a confusing watchdog message and every retry afterwards claims a load is still running.

Effect

Separation stays unavailable until the app is restarted, and the error text actively misleads: it tells the user to wait for a load that is not running.

Fix direction (not applied)

Release the latch on unwind — an RAII guard whose Drop clears in_progress, or catch_unwind around the load — so a panicking load reports an artifact failure and leaves recovery possible.

Not fixing here

Filed for tracking only.

Sign in to comment.

Linked pull requests

No linked pull requests yet.