dfinity / cdk-rs

Rust canister development kit for the Internet Computer.
Apache License 2.0
201 stars 89 forks source link

fix: trap with 'Call already trapped' when using futures::stream #450

Open mraszyk opened 11 months ago

mraszyk commented 11 months ago

This MR fixes the following issue with polling multiple futures using futures::stream: when invoking the method bar of the following canister code

async fn status() {
    let arg = CanisterIdRecord {
        canister_id: ic_cdk::id(),
    };
    let bytes = Encode!(&arg).unwrap();
    let _raw_resp = call_raw(
        Principal::management_canister(),
        "canister_status",
        bytes,
        0,
    )
    .await;
    ic_cdk::trap("trapping");
}

#[ic_cdk::update]
async fn bar() {
    let mut futs = vec![];
    for _ in 0..3 {
        futs.push(status());
    }
    let stream = futures::stream::iter(futs).buffer_unordered(10);
    stream.collect::<Vec<_>>().await;
}

the call fails with "Canister did not reply to the call" instead of the expected "Call already trapped".

This is the root cause I believe. Since the current context's waker (a.k.a. cdk-rs' waker) is replaced by another one, the new waker will be invoked here or here and then the new waker will wake the cdk-rs' waker (see here). Afterwards, it's the cdk-rs' waker responsibility to poll again. However, we don't want to poll again after a trap and thus the cdk-rs' waker will never be woken again after a trap and thus we won't keep trapping for subsequent callbacks. Consequently, the overall update call will fail with with "Canister did not reply to the call" instead of "Call already trapped". Moreover, the state of the outstanding futures will be dropped after the cleanup callback of the trapped future (since we won't poll the parent update call's future) and thus we can trap with "Call already trapped" if we don't find the call future's state in the callback handler which is exactly what this PR implements.