Add lockdown semantics to CanonicalABI.md - #728
Open
lukewagner wants to merge 1 commit into
Open
lukewagner wants to merge 1 commit into
lukewagner wants to merge 1 commit into
Conversation
This file contains hidden or 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
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.
As pointed out in #727, the "lockdown after trap" described in Explainer.md#component-invariants isn't defined by CanonicalABI.md, even though it should be. Although the current spec-text describes this as being a flag on a component instance, when a component A directly contains/imports a component B (via CM linking) and B traps, we don't want A sitting around alive in a corrupt state where various handles/functions implemented by B are now invalid, so A and B need to be in the same blast zone. Today the next thing "bigger" than a component instance is a
Store, and so this PR puts thelockdownflag onStoreand updates the verbiage accordingly. IIUC, this is also the unit-of-lockdown in Wasmtime.In a browser setting, I think this means that each instantiation of a root component via JS-API or ESM-integration semantically goes into a fresh
Store. IIUC, this aligns well with #686 where, if one component imports another through the JS-API or ESM-integration, there is no semantic special-case to "punch a hole" through the JS-API (like there is in core wasm) and thus there's no reason why two root components would ever need to be in the same store since they're always semantically calling each other through the host (which is allowed to call between stores).One other detail is that the lockdown is specified as
assert(not store.lockdown)instead oftrap_if(store.lockdown). This is mostly for the benefit ofStore.tickwhich describes execution from the event loop; in this case you don't want a trap/exception, you want to host to have avoided executing code in the store in the first place. Theassert()thus makes it the host's responsibility/choice rather than fixing the trap.Happy to discuss these details though. PTAL @eqrion