fix(process): reject duplicate ProcessManager spawn ids #116

Closed
opened 2026-08-26 15:14:40 +01:00 by gronod · 1 comment
Owner
Field Value
Labels Kind/Bug, Priority/Low
Priority Low
Milestone v0.3.3 hotfix
Related #63, #84
Branch fix/process-duplicate-id (from development)
Pair with

Description

ProcessManager::spawn inserts stdins[id] and killers[id] without checking occupancy. If chartread_${basename} is started twice (double-click Start, or Measure Another Sheet before exit is reaped):

  • The old Child wait task still runs.
  • The new stdin/killer replace the map entries.
  • When the old wait task exits, it remove()s the new id from both maps, so Calibrate / Skip / Cancel on the live process fail with Process not found.

Flagged since v0.2.1. More likely now that averaging reuses the same id.

Current behaviour

{
    let mut stdins = self.stdins.lock().await;
    stdins.insert(id.clone(), stdin_arc);
    let mut killers = self.killers.lock().await;
    killers.insert(id.clone(), kill_tx);
}

No occupancy check. Frontend #btnStartRead is shown again in FINISHED and can be clicked while a child is still shutting down.

Proposed solution

At the start of spawn:

{
    let stdins = self.stdins.lock().await;
    if stdins.contains_key(&id) {
        return Err(format!("Process '{id}' is still running"));
    }
}

Prefer a hard error over silently killing the occupant. Let the UI disable Start while currentState is not IDLE or FINISHED.

Frontend: disable #btnStartRead and #btnMeasureAnotherSheet until process:exit for that id.

Add a #[tokio::test] that:

  1. Spawns a long-lived process (sleep 30 or cat with piped stdin).
  2. Asserts a second spawn with the same id returns Err.
  3. Writes stdin successfully to the first child.
  4. Kills and asserts the exit event and that the maps are empty.
  5. Asserts the same id can then be reused.

Files

  • src-tauri/src/process_manager.rs
  • src/js/chartread.js — button disable

Acceptance criteria

  • Second spawn with the same id returns a clear error; first child still receives stdin.
  • After exit, the same id can be reused.
  • Tokio test covers spawn / stdin-while-wait / kill / respawn.
  • Start and Measure Another Sheet cannot be clicked while a chartread child is live.

Dependencies

None. Complements issue 01 but can land separately. If issue 01 lands first, this is still required as a backend guard.

| Field | Value | |---|---| | Labels | `Kind/Bug`, `Priority/Low` | | Priority | Low | | Milestone | v0.3.3 hotfix | | Related | #63, #84 | | Branch | `fix/process-duplicate-id` (from `development`) | | Pair with | — | ## Description `ProcessManager::spawn` inserts `stdins[id]` and `killers[id]` without checking occupancy. If `chartread_${basename}` is started twice (double-click Start, or Measure Another Sheet before exit is reaped): - The old `Child` wait task still runs. - The new stdin/killer replace the map entries. - When the **old** wait task exits, it `remove()`s the **new** id from both maps, so Calibrate / Skip / Cancel on the live process fail with `Process not found`. Flagged since v0.2.1. More likely now that averaging reuses the same id. ## Current behaviour ```rust { let mut stdins = self.stdins.lock().await; stdins.insert(id.clone(), stdin_arc); let mut killers = self.killers.lock().await; killers.insert(id.clone(), kill_tx); } ``` No occupancy check. Frontend `#btnStartRead` is shown again in `FINISHED` and can be clicked while a child is still shutting down. ## Proposed solution At the start of `spawn`: ```rust { let stdins = self.stdins.lock().await; if stdins.contains_key(&id) { return Err(format!("Process '{id}' is still running")); } } ``` Prefer a hard error over silently killing the occupant. Let the UI disable Start while `currentState` is not `IDLE` or `FINISHED`. Frontend: disable `#btnStartRead` and `#btnMeasureAnotherSheet` until `process:exit` for that id. Add a `#[tokio::test]` that: 1. Spawns a long-lived process (`sleep 30` or `cat` with piped stdin). 2. Asserts a second `spawn` with the same id returns `Err`. 3. Writes stdin successfully to the first child. 4. Kills and asserts the exit event and that the maps are empty. 5. Asserts the same id can then be reused. ## Files - `src-tauri/src/process_manager.rs` - `src/js/chartread.js` — button disable ## Acceptance criteria - [ ] Second spawn with the same id returns a clear error; first child still receives stdin. - [ ] After exit, the same id can be reused. - [ ] Tokio test covers spawn / stdin-while-wait / kill / respawn. - [ ] Start and Measure Another Sheet cannot be clicked while a chartread child is live. ## Dependencies None. Complements issue 01 but can land separately. If issue 01 lands first, this is still required as a backend guard.
gronod added this to the v0.3.3 Hot fixes milestone 2026-08-26 15:14:40 +01:00
gronod added the Kind/Bug
Reviewed
Confirmed
1
Priority
Low
4
labels 2026-08-26 15:14:40 +01:00
gronod changed title from # fix(process): reject duplicate ProcessManager spawn ids to fix(process): reject duplicate ProcessManager spawn ids 2026-08-27 15:31:31 +01:00
Author
Owner

Resolved via PR #123. ProcessManager::spawn now returns a hard error if an existing process with the same ID is actively registered in stdins.

Resolved via PR #123. `ProcessManager::spawn` now returns a hard error if an existing process with the same ID is actively registered in `stdins`.
Sign in to join this conversation.