fix(execution): Canister snapshots should include Wasm globals #1250
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
luc-blaeser
changed the title
github-actions
bot
added
@ic-message-routing-owners
@execution
@ic-interface-owners
labels
Why do you think so? |
Aren't they also extracted from the wasm and should therefore be restored when the wasm is restored? I might be mistaken of course, you know that code better. |
I believe the exports and metadata should be restored from the restored Wasm module. And I guess that's why we missed globals... |
I got it wrong then and it's all good. |
berestovskyy
approved these changes
Sep 2, 2024
rs/execution_environment/src/execution_environment/tests/canister_snapshots.rs
Outdated
Show resolved
Hide resolved
schneiderstefan
approved these changes
Sep 2, 2024
dsarlis
approved these changes
Sep 3, 2024
mergify bot
pushed a commit
to dfinity/motoko
that referenced
this pull request
Sep 9, 2024
Using IC 2024-08-16 with some adjustments: * `drun` adjustments for better testing - dfinity/ic#662 (Lift 8GB memory boundary from drun) - dfinity/ic#988 (Enable Wasm64 with 16GB main memory capacity) - dfinity/ic#991 (Disable DTS for deterministic debug outputs) - dfinity/ic#992 (Increase batch limit for longer running tests) - dfinity/ic#1240 (Enable canister snapshots) * IC build fixes for `nix`: - Remove duplicate entries of same crate with same versions in `Cargo.toml`. - dfinity/ic#993 (Fix MacOS `rocksdb` dependency). * Other IC fixes: - dfinity/ic#1250 (Fix canister snapshots)
levifeldman
pushed a commit
to levifeldman/ic
that referenced
this pull request
Oct 1, 2024
…ty#1250) Currently, the canister snapshots do not cover the state of Wasm global variables, such that in realistic canister programs with mutable global variables, the state is not properly recovered when a snapshot is loaded. This fix adds the Wasm globals to the snapshot state, similar to the execution state. (Side note: All Wasm variables are exported by the Wasm instrumentation.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the canister snapshots do not cover the state of Wasm global variables, such that in realistic canister programs with mutable global variables, the state is not properly recovered when a snapshot is loaded.
This fix adds the Wasm globals to the snapshot state, similar to the execution state. (Side note: All Wasm variables are exported by the Wasm instrumentation.)