coordinator: stale PendingRestart leaks reply_sender and hangs caller if a daemon dies mid-restart #2131

Activity

1. phil-opp

   added

   bugSomething isn't workingSomething isn't working

   on Jun 10, 2026
2. github-actions

   added

   cliCLICLI

   coordinator

   daemon

   documentationImprovements or additions to documentationImprovements or additions to documentation

   rust

   on Jun 10, 2026
3. phil-opp commented on Jun 16, 2026

   

   phil-opp

   on Jun 16, 2026 – with Claude

   CollaboratorAuthor

   More actions

   Investigating this as part of an automated issue review pass — the three problems described here all appear to already be resolved in the current main:

   Problem 1 (caller hangs on daemon crash mid-restart): cleanup_disconnected_daemons_from_running_dataflows already drains pending_restarts for all affected dataflows and fires the reply_sender with an error (coordinator/src/lib.rs:3046–3058). The inline TODO is gone.

   Problem 2 (double-restart guard runs after StopDataflow): initiate_restart already checks pending_restarts.contains_key(&dataflow_uuid) as the first thing it does, before any stop_dataflow call (coordinator/src/lib.rs:4624–4631).

   Problem 3 (no coordinator test): Two tests exist:

   • cleanup_disconnected_daemons_drains_pending_restarts — verifies the stale entry is removed and the caller is released with an error (lib.rs:7833)
   • initiate_restart_rejects_duplicate_request — verifies the double-restart guard (lib.rs:7881)

   Both pass under cargo test -p dora-coordinator. This issue can likely be closed unless there are additional cases not captured here.

   ---

   Generated by Claude Code
4. phil-opp commented on Jun 18, 2026

   

   phil-opp

   on Jun 18, 2026 – with Claude

   CollaboratorAuthor

   More actions

   After reviewing the current codebase, both problems described here are already addressed:

   Problem 1 — daemon crash during restart hangs caller

   Fixed in binaries/coordinator/src/lib.rs around line 3046 (cleanup_disconnected_daemons_from_running_dataflows). When a daemon disconnects, the function drains pending_restarts for every affected dataflow UUID and fires reply_sender with an error:

   for uuid in &affected_uuids {
       if let Some(restart) = pending_restarts.remove(uuid) {
           tracing::warn!(dataflow = %uuid, "daemon disconnected while restart was pending; cancelling restart");
           let _ = restart.reply_sender.send(Err(eyre!(
               "daemon disconnected while restart was pending for dataflow `{uuid}`"
           )));
       }
   }

   This is invoked from the Event::DaemonExit handler (around line 2209), so a daemon crash/disconnect during Phase 1 now returns an error to the CLI caller instead of hanging.

   Problem 2 — double-restart guard

   Present at binaries/coordinator/src/lib.rs around line 4624, placed before any side effects in initiate_restart:

   if pending_restarts.contains_key(&dataflow_uuid) {
       let _ = reply_sender.send(Err(eyre!(
           "dataflow `{dataflow_uuid}` is already being restarted – wait for it to finish"
       )));
       return;
   }

   Tests

   Both behaviors are covered by unit tests in the same file:

   • cleanup_disconnected_daemons_drains_pending_restarts (Problem 1)
   • initiate_restart_rejects_duplicate_request (Problem 2)

   It looks like these fixes landed after this issue was filed. This issue can be closed.

   ---

   Generated by Claude Code
5. phil-opp commented on Jun 22, 2026

   

   phil-opp

   on Jun 22, 2026 – with Claude

   CollaboratorAuthor

   More actions

   All three problems described in this issue have been addressed in the codebase:

   Problem 1 (caller hangs on daemon disconnect): cleanup_disconnected_daemons_from_running_dataflows now drains matching pending_restarts entries and fires the reply with an error when a daemon disconnects mid-restart (lib.rs:3046–3059). The inline TODO referenced in the issue has been removed.

   Problem 2 (double-restart guard fires after StopDataflow): The pending_restarts.contains_key(...) guard was moved to the top of initiate_restart, before stop_dataflow is called (lib.rs:4626–4631).

   Problem 3 (no test): Two unit tests cover the new behavior:

   • cleanup_disconnected_daemons_drains_pending_restarts (lib.rs:7833) — verifies the caller receives an Err on daemon disconnect.
   • initiate_restart_rejects_duplicate_request (lib.rs:7881) — verifies duplicate restarts are rejected before any daemon I/O.

   This issue can be closed.

   ---

   Generated by Claude Code
6. phil-opp commented on Jun 27, 2026

   

   phil-opp

   on Jun 27, 2026 – with Claude

   CollaboratorAuthor

   More actions

   After reviewing the current codebase, all three problems described in this issue appear to already be fixed:

   Problem 1 (caller hangs forever on daemon disconnect) — Fixed in cleanup_disconnected_daemons_from_running_dataflows (binaries/coordinator/src/lib.rs). When a daemon disconnects, the function now drains any matching pending_restarts entry and fires the reply sender with an error:

   // Drain pending restarts for affected dataflows: the disconnected
   // daemon(s) will never send DataflowFinishedOnDaemon, so any caller
   // waiting on a deferred restart would hang indefinitely (#2082 H1).
   for uuid in &affected_uuids {
       if let Some(restart) = pending_restarts.remove(uuid) {
           tracing::warn!(dataflow = %uuid, "daemon disconnected while restart was pending; cancelling restart");
           let _ = restart.reply_sender.send(Err(eyre!(
               "daemon disconnected while restart was pending for dataflow `{uuid}`"
           )));
       }
   }

   Problem 2 (double-restart guard runs after StopDataflow) — Fixed in initiate_restart. The pending_restarts.contains_key(...) guard is now checked at the top of the function, before any daemon I/O:

   // Guard against double-restart FIRST: reject duplicate requests before
   // touching any daemon state or sending StopDataflow side effects.
   if pending_restarts.contains_key(&dataflow_uuid) {
       let _ = reply_sender.send(Err(eyre!("dataflow `{dataflow_uuid}` is already being restarted – wait for it to finish")));
       return;
   }

   Problem 3 (no coordinator-side test) — Two tests now cover these paths: cleanup_disconnected_daemons_drains_pending_restarts and initiate_restart_rejects_duplicate_request.

   Closing as already resolved.

   ---

   Generated by Claude Code
7. phil-opp commented on Jul 10, 2026

   

   phil-opp

   on Jul 10, 2026 – with Claude

   CollaboratorAuthor

   More actions

   All three problems in this issue appear to be already resolved on main (checked at caf62a0) — no code change is needed. Details:

   Problem 1 (primary) — caller hangs if a daemon dies mid-restart: the daemon-disconnect cleanup now drains matching pending_restarts entries and replies Err so the caller gets a clear error instead of hanging. See cleanup_disconnected_daemons_from_running_dataflows in binaries/coordinator/src/lib.rs:

   // Drain pending restarts for affected dataflows: the disconnected
   // daemon(s) will never send DataflowFinishedOnDaemon, so any caller
   // waiting on a deferred restart would hang indefinitely (#2082 H1).
   for uuid in &affected_uuids {
       if let Some(restart) = pending_restarts.remove(uuid) {
           tracing::warn!(dataflow = %uuid,
               "daemon disconnected while restart was pending; cancelling restart");
           let _ = restart.reply_sender.send(Err(eyre!(
               "daemon disconnected while restart was pending for dataflow `{uuid}`")));
       }
   }

   The inline TODO referenced in the issue is gone.

   Problem 2 — double-restart guard runs after StopDataflow: in initiate_restart the pending_restarts.contains_key(...) guard is now checked first, before extracting the descriptor and before stop_dataflow, so a duplicate Restart is rejected before any daemon I/O:

   // Guard against double-restart FIRST: reject duplicate requests before
   // touching any daemon state or sending StopDataflow side effects.
   if pending_restarts.contains_key(&dataflow_uuid) {
       let _ = reply_sender.send(Err(eyre!(
           "dataflow `{dataflow_uuid}` is already being restarted – wait for it to finish")));
       return;
   }

   Problem 3 — no coordinator-side test: coverage now exists in binaries/coordinator/src/lib.rs tests:

   • cleanup_disconnected_daemons_drains_pending_restarts (locks in the Problem 1 cleanup path)
   • initiate_restart_rejects_duplicate_request (locks in the Problem 2 guard)

   This issue looks safe to close. Flagging rather than opening a PR since there's nothing left to change.

   _{Filed automatically by a Claude code-review check; please verify before closing.}

   ---

   Generated by Claude Code
8. phil-opp

   added 2 commits that reference this issue on Jul 17, 2026

   test(coordinator): cover pending-restart cleanup on daemon disconnect

   ...

   809a466

   test(coordinator): cover pending-restart cleanup on daemon disconnect

   ...

   e289888
9. phil-opp

   closed this as completedon Aug 4, 2026