Hi Victoria, Daniel,
We build a DAO framework outside the SNS stack and we recently had to answer the same question for ourselves: must a governance canister stop a canister before upgrading it?
We ended up reading the SNS implementation and then measuring the underlying hazard, so here are four things that might be useful. Everything below is either quoted from dfinity/ic or measured, and I have flagged the one place where we are inferring.
1. The deadlock is one specific loop, and it is easy to point at
rs/sns/governance/src/canister_control.rs, tail of stop_canister:
// Wait until canister is in the stopped state.
loop {
let status = canister_status(env, canister_id).await?;
if status.status == CanisterStatusType::Stopped {
return Ok(());
}
log!(INFO, "{}Still waiting for canister {} to stop. status: {:?}",
log_prefix(), canister_id, status);
}
No timeout, no iteration cap. And each canister_status poll is itself an open call context on the poller, which is the part that closes the trap: a canister sitting in this loop can never reach Stopped itself, because it always has a call in flight. That is exactly the Root/Governance situation Daniel described, expressed in code.
2. The serialisation is broader than the hazard it guards against.
The mutual-stop cycle needs two canisters that each control the other. Two dapp canisters that do not control one another cannot form that cycle no matter how concurrently they are upgraded, which is precisely Victoria’s case: different targets, no relationship between them.
So a serialisation keyed on “does this upgrade involve the mutually-controlling pair” would keep the guarantee and unblock the common case. Daniel’s own caveat in post #4 points the same way.
(Note: Since rs/sns/governance/src/governance.rs exceeds the scope of our local tooling, the point above is inferred from the thread, the error text, and commit NNS1-1108. If the check is already implemented with a narrower scope than global, please disregard this point.)
3. There is a way to keep the stop and lose the deadlock: break the initiator’s call context.
The reason the initiator cannot be stopped is that it is holding a call open while it waits. If instead it dispatches the stop-and-upgrade work into a fresh message and returns immediately, its own context closes and it becomes stoppable. The outcome is then observed through state rather than through a reply.
Concretely, one shape that works:
-
the initiator records “workflow W started for target T” in stable memory and returns;
-
a separate, detached message performs stop, install, start;
-
completion is reported back through an explicit call, and an interrupted workflow is reconciled from the stable record on the next upgrade or by a sweep.
We run this pattern in production for self-targeted lifecycle work, for the same reason: the canister that asks cannot also be the canister that waits. It costs an idempotency key and a reconciliation path, and it removes the need for a global lock rather than mitigating it.
Two smaller things that helped us in the same area: bound the stop call rather than polling forever, so a wedged target produces a failed proposal instead of an unrecoverable one, and give the target a chance to quiesce first, since a target that keeps starting new outbound calls is a target that never reaches Stopped.
4. We measured the rationale for stopping, and on ic-cdk 0.19 it does not reproduce.
The stated reason to stop is in rs/nervous_system/root/src/change_canister.rs, on ChangeCanisterRequest::stop_before_installing:
Rust
/// The value depend on the canister. For instance:
/// * Canisters that don't emit any inter-canister call, such as the
/// registry canister, have no reason to be stopped before being upgraded.
/// * Canisters that emit inter-canister call are at risk of undefined
/// behavior if a callback is delivered to them after the upgrade.
The sharpest description of that “undefined behavior” is Joachim Breitner’s, from 2021-11-09: the callback “would eventually reach the new instance, and because of infelicities of the IC’s System API, could possibly call arbitrary internal functions”.
That is nearly five years old, so we tested it on PocketIC. Caller and callee are two instances of one probe; the callee holds its reply open across many rounds; the caller is upgraded to a structurally different module while awaiting; then the reply is released. Both unbounded_wait and bounded_wait.
Result, identical in both modes:
| observable |
outcome |
| response delivered to the upgraded canister |
yes |
| old continuation resumed |
no |
| any uncalled function executed |
no |
| caller alive afterwards |
yes |
| stable memory intact |
yes |
| ingress caller’s result |
clean reject, CanisterCalledTrap |
ic-cdk traps on purpose rather than dispatching anywhere:
Panicked at 'internal error: entered unreachable code: CallFutureState for
in-flight calls should only be Executing or Trapped (callback)',
ic-cdk-0.19.0/src/call.rs:984:13
call_on_cleanup traps too, so a Drop-based guard does not run in that path; worth knowing for anyone relying on RAII release across an upgrade.
The version caveat, which matters here. We measured ic-cdk 0.19.0. SNS pins ic-cdk 0.20.2 (Cargo.toml, [workspace.dependencies]), and the guard that produced this trap lives in ic-cdk’s own call.rs exactly the kind of code that moves between minor versions. Same crate and same Call API on both sides, one minor version apart, so we would expect the same behaviour, but we have not measured 0.20.2 and are not claiming it.
If that carries to 0.20.2, then for SNS-controlled dapp canisters the stop is buying less than it did in 2021: the failure mode is a clean reject and a half-finished flow to reconcile, rather than the arbitrary dispatch the comment warns about. It would not change anything for Root and Governance, where the stop exists for a different reason.
What we would find useful, if anyone at dfinity has the context: is stop_before_installing still considered load-bearing for dapp canisters on current ic-cdk, or is the trap now the intended behaviour and the stop mostly historical? The answer changes the design for anyone building governance outside the SNS framework, and right now it is only recoverable by measuring.
Happy to share the probe if it is useful; it is about forty lines plus a PocketIC driver. 