Skip to content

Commit

Permalink
test: testing ability to upgrade after trapping finally (#4641)
Browse files Browse the repository at this point in the history
A trapping finally block will (necessarily) leak a callback table slot.

Motoko prevents a canister with non-empty callback table from upgrading, but only unless the canister is stopped.

This test verifies that we can recover from a callback table leak (due a faulty finally) by first stopping the canister, upgrading and restarting it.

This PR also adds an admonition documenting the danger of trapping finally-blocks and how to recover from them.
  • Loading branch information
crusso authored Jul 31, 2024
1 parent 9badafc commit 22804f9
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 1 deletion.
14 changes: 13 additions & 1 deletion doc/md/reference/language-manual.md
Original file line number Diff line number Diff line change
Expand Up @@ -2550,11 +2550,23 @@ Because the [`Error`](../base/Error.md) type is opaque, the pattern match cannot

:::

The `try` expression can be provided with a `finally` cleanup clause to facilitate structured rollback of temporary state changes (e.g. to release a lock).
The `try` expression can be provided with a `finally` cleanup clause to facilitate structured rollback of temporary state changes (e.g. to release a lock).
The preceding `catch` clause may be omitted in the presence of a `finally` clause.

This form is `try <block-or-exp1> (catch <pat> <block-or-exp2>)? finally <block-or-exp3>`, and evaluation proceeds as above with the crucial addition that every control-flow path leaving `<block-or-exp1>` or `<block-or-exp2>` will execute the unit-valued `<block-or-exp3>` before the entire `try` expression produces its result. The cleanup expression will additionally also be executed when the processing after an intervening `await` (directly, or indirectly as `await*`) traps.

:::danger

The code within a `finally` block should terminate promptly and not trap.
A trapping finally block will fail to free its callback table slot which
can prevent a future upgrade.
In this situation, the canister should be explicitly stopped before re-attempting the upgrade.
In addition, care should be taken to release any resources that may have remained acquired due to the trap.
The canister may be re-started after the upgrade.

:::


See [Error type](#error-type).

### Assert
Expand Down
13 changes: 13 additions & 0 deletions test/run-drun/ok/try-finally-trap-stop.drun-run.ok
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
ingress Completed: Reply: 0x4449444c0000
debug.print: {rts_callback_table_count = 0}
debug.print: trap in finally!
debug.print: trap in finally!
debug.print: {rts_callback_table_count = 1}
debug.print: canister running
debug.print: Canister rrkah-fqaaa-aaaaa-aaaaq-cai trapped explicitly: canister_pre_upgrade attempted with outstanding message callbacks (try stopping the canister before upgrade)
debug.print: canister stopped
debug.print: canister upgraded
debug.print: {rts_callback_table_count = 0}
debug.print: done
ingress Completed: Reply: 0x4449444c0000
14 changes: 14 additions & 0 deletions test/run-drun/ok/try-finally-trap.drun.ok
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
ingress Completed: Reply: 0x4449444c0000
ingress Completed: Reply: 0x4449444c0000
debug.print: trap in finally!
ingress Err: IC0503: Canister rwlgt-iiaaa-aaaaa-aaaaa-cai trapped explicitly: assertion failed at try-finally-trap.mo:8.7-8.19

call_on_cleanup also failed:

Canister rwlgt-iiaaa-aaaaa-aaaaa-cai trapped explicitly: assertion failed at try-finally-trap.mo:12.7-12.19
debug.print: {rts_callback_table_count = 1}
ingress Completed: Reply: 0x4449444c0000
ingress Err: IC0503: Canister rwlgt-iiaaa-aaaaa-aaaaa-cai trapped explicitly: canister_pre_upgrade attempted with outstanding message callbacks (try stopping the canister before upgrade)
debug.print: {rts_callback_table_count = 1}
ingress Completed: Reply: 0x4449444c0000
68 changes: 68 additions & 0 deletions test/run-drun/try-finally-trap-stop.mo
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import Lib "try-finally-trap-stop/class";
import Prim "mo:⛔";

// tests that a canister with a leaky callback table
// (due to a trapping finally block) preventing upgrades,
// can still be upgraded after being stopped.
actor {

let ic00 = actor "aaaaa-aa" : actor {
stop_canister : { canister_id : Principal } -> async ();
start_canister : { canister_id : Principal } -> async ();
};

public func go() : async () {
let pre = Prim.cyclesBalance();
// instantiate Burner actor
Prim.cyclesAdd<system>(pre/2);
var a = await Lib.C();
let p = Prim.principalOfActor(a);
a := await (system Lib.C) (#upgrade a) ();
await a.show();
try {
await a.leak(); // leaks a callback in a's callback table
} catch _ {
};
await a.show();
Prim.debugPrint("canister running");
try {
a := await (system Lib.C) (#upgrade a) ();
Prim.debugPrint("canister upgraded");
assert false;
} catch e {
Prim.debugPrint(Prim.errorMessage (e));
};
try {
await ic00.stop_canister({canister_id = p});
}
catch e {
Prim.debugPrint(Prim.errorMessage (e));
assert false;
};
Prim.debugPrint("canister stopped");
try {
a := await (system Lib.C) (#upgrade a) ();
Prim.debugPrint("canister upgraded");
} catch e {
Prim.debugPrint(Prim.errorMessage (e));
assert false;
};

try {
await ic00.start_canister({canister_id = p});
}
catch e {
Prim.debugPrint(Prim.errorMessage (e));
assert false;
};
await a.show();
Prim.debugPrint("done");

}

}

//SKIP ic-ref-run
//SKIP run-low
//SKIP run-ir
//CALL ingress go "DIDL\x00\x00"
23 changes: 23 additions & 0 deletions test/run-drun/try-finally-trap-stop/class.mo
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { debugPrint; rts_callback_table_count } = "mo:⛔";

actor class C() {

// leak a callback_table_slot by trapping in finally
public func leak() : async () {
try {
await async ();
}
finally {
debugPrint("trap in finally!");
assert false
};
};

public func show() : async () {
debugPrint(debug_show
{ rts_callback_table_count = rts_callback_table_count()
});
};

};

6 changes: 6 additions & 0 deletions test/run-drun/try-finally-trap.drun
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
install $ID try-finally-trap/try-finally-trap.mo ""
upgrade $ID try-finally-trap/try-finally-trap.mo ""
ingress $ID go "DIDL\x00\x00"
ingress $ID show "DIDL\x00\x00"
upgrade $ID try-finally-trap/try-finally-trap.mo ""
ingress $ID show "DIDL\x00\x00"
23 changes: 23 additions & 0 deletions test/run-drun/try-finally-trap/try-finally-trap.mo
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { debugPrint; rts_callback_table_count } = "mo:⛔";

actor A {

public func go() : async () {
try {
await async ();
assert false
}
finally {
debugPrint("trap in finally!");
assert false
};
};

public func show() : async () {
debugPrint(debug_show
{ rts_callback_table_count = rts_callback_table_count() });
};


};

0 comments on commit 22804f9

Please sign in to comment.