Skip to content

Add lockdown semantics to CanonicalABI.md - #728

Open
lukewagner wants to merge 1 commit into
mainfrom
lockdown
Open

lukewagner wants to merge 1 commit into
mainfrom
lockdown

Conversation

@lukewagner

Copy link
Copy Markdown
Member

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 the lockdown flag on Store and 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 of trap_if(store.lockdown). This is mostly for the benefit of Store.tick which 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. The assert() thus makes it the host's responsibility/choice rather than fixing the trap.

Happy to discuss these details though. PTAL @eqrion

@eqrion eqrion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants