Conversation
chrisdickinson
left a comment
chrisdickinson
left a comment
There was a problem hiding this comment.
Oof, that's a tricky one! I left one comment to address – zeroing out self.host_context when we don't receive a host context – but otherwise I think this LGTM!
| let foo = current_plugin.host_context::<Foo>()?.clone(); | ||
| let hnd = current_plugin.memory_new(foo.message)?; | ||
| ret[0] = current_plugin.memory_to_val(hnd); | ||
| Ok(()) |
There was a problem hiding this comment.
The invariant that's on my mind: if I call_with_host_context(host context = A) and then call(), will host functions invoked by the second call see A as their context?
There was a problem hiding this comment.
The host context should be reset before the next call (see the comment below for the link)
| } | ||
| Some(self.host_context.clone()) | ||
| } else { | ||
| None |
There was a problem hiding this comment.
I think we may want to zero out self.host_context here?
There was a problem hiding this comment.
I made it so we're zeroing out the host context after each call: https://github.com/extism/extism/pull/759/files/be744b5ea7df5c1a6f3d5accb28324c811a71e73#diff-fc3b26932af6a72d6a908c3d72663d22045526b2c46c6777aadeb7852d1137d4R816 - but am open to moving it up to here if you think that makes more sense.
There was a problem hiding this comment.
Oh, perfect! That makes sense.
2 participants
Updates plugins to allocate a single
ExternReffor the host context up front, to avoid running into thefailed to allocate externreferror from Wasmtime