Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Adds a test to ensure that we clear the heap between calls into runtime #4903

Merged
merged 3 commits into from
Feb 14, 2020

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Feb 12, 2020

The tests shows that we currently not clearing the heap in wasmtime.
For now we don't run the test for wasmtime.

The tests shows that we currently not clearing the heap in wasmtime.
For now we don't run the test for wasmtime.
@bkchr bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes labels Feb 12, 2020
@bkchr bkchr requested a review from pepyakin as a code owner February 12, 2020 11:35
@@ -34,4 +34,7 @@ pub trait WasmRuntime {

/// Call a method in the Substrate runtime by name. Returns the encoded result on success.
fn call(&mut self, method: &str, data: &[u8]) -> Result<Vec<u8>, Error>;

/// Get the value from a global with the given `name`.
fn get_global_val(&mut self, name: &str) -> Result<Option<Value>, Error>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tend not use get_ prefix. Rename it to global_val perhaps?

@arkpar
Copy link
Member

arkpar commented Feb 13, 2020

Is this really intended behaviour? I'd expect runtime instance to keep all of its state between calls. This might not be what we want in substrate, but it makes more sense for wasmi in general. It would make more sense to have an explicit clear_heap() or even reset() that would reset not only heap, but global variables as well.

@bkchr
Copy link
Member Author

bkchr commented Feb 13, 2020

Is this really intended behaviour? I'd expect runtime instance to keep all of its state between calls. This might not be what we want in substrate, but it makes more sense for wasmi in general. It would make more sense to have an explicit clear_heap() or even reset() that would reset not only heap, but global variables as well.

Yes that is intended behavior, we clear the memory between calls.

@bkchr bkchr merged commit d02c720 into master Feb 14, 2020
@bkchr bkchr deleted the bkchr-check-heap-is-cleared branch February 14, 2020 00:42
General-Beck pushed a commit to General-Beck/substrate that referenced this pull request Feb 18, 2020
…me (paritytech#4903)

* Adds a test to ensure that we clear the heap between calls into runtime

The tests shows that we currently not clearing the heap in wasmtime.
For now we don't run the test for wasmtime.

* Fix compilation
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants