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

Cannot leave component instance issue #97

Open
Mossaka opened this issue Feb 28, 2023 · 14 comments
Open

Cannot leave component instance issue #97

Mossaka opened this issue Feb 28, 2023 · 14 comments

Comments

@Mossaka
Copy link
Member

Mossaka commented Feb 28, 2023

Hey, not sure if this is the right place to ask but the issue was caused when wasm32-wasi target is used in my case.

I tried to execute a wasi component using the host crate from this repo and I got a message showed below

(base) ➜  wasi-messaging-demo git:(main) ✗ make run 
Running...
    Finished dev [unoptimized + debuginfo] target(s) in 1.90s
     Running `target/debug/host`
Error: error while executing at wasm backtrace:
    0: 0x288de2 - wit-component:shim!indirect-wasi-environment-get-environment
    1: 0x2871b7 - wit-component:adapter:wasi_snapshot_preview1!wasi_snapshot_preview1::State::get_environment::h1684658e26d89eb6
    2: 0x2873ab - wit-component:adapter:wasi_snapshot_preview1!environ_sizes_get
    3: 0x288e0a - wit-component:shim!adapt-wasi_snapshot_preview1-environ_sizes_get
    4: 0x1d003 - <unknown>!__wasi_environ_sizes_get
    5: 0x1d06a - <unknown>!__wasilibc_initialize_environ
    6: 0x1d10a - <unknown>!__wasilibc_initialize_environ_eagerly
    7:  0x9a8 - <unknown>!__wasm_call_ctors
    8: 0x23a1f - <unknown>!cabi_realloc.command_export

Caused by:
    cannot leave component instance
make: *** [run] Error 1

The setup

I am using the 'release-6.0.0' branch of wasmtime and the latest commits of this repo and wit-bindgen, wasm-tools for creating a component out of my Rust application.

This is the output of wasm-tools component wit

...
default world guest.component {
  import messaging-types: self.messaging-types
  import producer: self.producer
  import wasi-stderr: self.wasi-stderr
  import wasi-io: self.wasi-io
  import wasi-filesystem: self.wasi-filesystem
  import wasi-exit: self.wasi-exit
  import wasi-environment: self.wasi-environment
  export handler: self.handler
}

My host is building a Wasi Context using wasi_cap_std_sync crate from this repo as well.

Getting Around

I was able to get around this error by compiling the application to wasm32-unknown-unknown target and remove --adapt flag from wasm-tools component command. This indicates that there might be something going on in wasi and thus I want to raise this issue here.

@alexcrichton
Copy link
Member

I don't know how this hasn't cropped up sooner, and unfortunately I'm not sure how best to solve this. The error here is coming from Wasmtime itself and is reflective of the reentrance rules of the component model. The realloc function, when called, isn't allowed to "leave" the component which it's doing so here via ctors acquiring environment variables. I'm not sure what the best solution to that is.

@dicej
Copy link
Collaborator

dicej commented Feb 28, 2023

@alexcrichton We could generalize the logic I added in #83 and bytecodealliance/wasm-tools#919 such that wit-component wraps the main module's cabi_realloc function in some code that sets the allocation_state global (and unsets it on exit) to a value that the Preview 2 adapter recognizes as meaning "short circuit any host calls". Currently we do that only when allocating the stack and adapter State, but no reason we can't do that for all cabi_realloc calls.

Does that sound reasonable? If so, I'll be happy to implement it.

@dicej
Copy link
Collaborator

dicej commented Feb 28, 2023

BTW, will this be an issue for post-return functions also?

@alexcrichton
Copy link
Member

This will be an issue with post-return, yeah, but #83 and related support was about updates to the adapter module but this is a case where cabi_realloc starts in the main module so technically, if we look a bit further in the future where wasi-libc is bounds to preview2 directly, the adapter can't solve this. I think it may be able to paper over things today with some really invasive surgery on the main module or the adapter (e.g. importing all reallocs/post-returns into the adapter module, doing some adaptation, and then using those as the lifting options rather than the original ones) as you mention though.

This may be a deeper issue worth talking about with @sunfishcode and @lukewagner though to see if there's a better solution here. I'm not sure how flexible this reentrancy constraint is or how flexible the env var/ctor code can be made.

@dicej
Copy link
Collaborator

dicej commented Feb 28, 2023

I discussed this with @sunfishcode and @lukewagner today, and I think the plan going forward is to move away from wrapping each export in ctor/dtor calls and instead rely on initialization happening in an _init (for reactors) or _start (for commands) function. @sunfishcode is going to work to get that standardized and official in the appropriate forums.

So I think we can handle the ctor/dtor wrappers with the preview2 adapter and then assume there will be no such wrappers by the time wasi-libc uses preview2 directly. Does that sound reasonable?

@pchickey
Copy link
Collaborator

pchickey commented Feb 28, 2023

Re relying on initialization happening in an _init for reactors - is this something the component model will support natively, i.e. this is run as part of instantiation of a component, versus being a component export function which needs to be executed before any other functions (and some external mechanism enforces this rule)?

Can you describe more what "handle the ctor/dtor wrappers with the preview2 adapter" means?

From my view (and I apologize that I came to this separately and didnt join the discussion you just had) this difficulty is driven by the adapter needing to rely on wasi-libc in order to allocate memory. This requirement is driven by needing to use "legacy" wasi-libcs that have the allocator behavior from before components or adapters were created.

If we can throw away that requirement, and require preview 1 apps be linked against a wasi-libc which won't stomp on the adapter's allocation, we can resolve this problem in a way that eliminates a lot of the complexity in the adapter. (The fixed wasi-libc allocator is available in rustc nightly, right @sunfishcode?) By "moving away from wrapping each export in ctor/dtor wrappers" we mean that some change to lld is required in order to produce modules that work with the adapter, then have we already made that leap?

@pchickey
Copy link
Collaborator

pchickey commented Feb 28, 2023

Alex corrected my understanding of this issue - even if we can separate allocation from the adapter and libc, we would still run into this issue because libc is calling import functions during the call to cabi_realloc (which is being called from outside the component in preparation for passing in some lists to an export function)

@pchickey
Copy link
Collaborator

pchickey commented Feb 28, 2023

How plausible is it to change wasi-libc just not call any import functions in its constructors? (Edit: I read all of Dan's comments throughout wasi-libc on environment initialization and read man 7 environ and now I am sad but resigned, there is no way out of calling import functions while initializing a working libc)

@dicej
Copy link
Collaborator

dicej commented Feb 28, 2023

(I'll let Luke or Dan answer your first question, since they floated a few ideas, and I'm not totally sure where they landed).

Can you describe more what "handle the ctor/dtor wrappers with the preview2 adapter" means?

I outlined it in my earlier comment: #97 (comment)

From my view (and I apologize that I came to this separately and didnt join the discussion you just had) but the lens I am looking at this is, this difficulty is driven by the adapter needing to rely on wasi-libc in order to allocate memory. This requirement is driven by needing to use "legacy" wasi-libcs that have the allocator behavior from before components or adapters were created. If we can throw away that requirement, and require preview 1 apps be linked against a wasi-libc which won't stomp on the adapter's allocation, we can resolve this problem in a way that eliminates a lot of the complexity in the adapter. (The fixed wasi-libc allocator is available in rustc nightly, right @sunfishcode?)

At Fermyon, we would prefer to have a way to convert customers' existing modules to components without rebuilding from source. If that is prohibitively difficult then we can explore other options, but I don't think that's a given at this point. Also, some toolchains, such as TeaVM, don't use wasi-libc at all, and will require modifications to accommodate a "reserved" portion of linear memory for the adapter. That could get awkward as more non-wasi-libc tools come on the scene. And as Alex pointed out, requiring the latest wasi-libc wouldn't help with this particular issue anyway.

@dicej
Copy link
Collaborator

dicej commented Feb 28, 2023

How plausible is it to change wasi-libc just not call any import functions in its constructors?

Probably doable for environment variables, but not sure about preopens.

@pchickey
Copy link
Collaborator

Can you describe more what "handle the ctor/dtor wrappers with the preview2 adapter" means?

I outlined it in my earlier comment: #97 (comment)

I don't understand how this approach is viable, because short-circuiting the response to wasi-libc gives it incorrect values. How do those values later get corrected when calling imports is allowed?

@dicej
Copy link
Collaborator

dicej commented Feb 28, 2023

I don't understand how this approach is viable, because short-circuiting the response to wasi-libc gives it incorrect values. How do those values later get corrected when calling imports is allowed?

The incorrect values will only live persist as long as the cabi_realloc call is happening. Later calls to exports will run all the constructors again, and this time they'll get the correct values. Currently, wasi-libc's constructors always assume they're being called for the first time, so there's no boolean check to see if they've already run. That happens to be to our advantage in this case.

@alexcrichton
Copy link
Member

So I think we can handle the ctor/dtor wrappers with the preview2 adapter and then assume there will be no such wrappers by the time wasi-libc uses preview2 directly. Does that sound reasonable.

That sounds reasonable to me as a stopgap, yeah. (albeit this continues to pile quite a few hacks in the adapter construction but our hand is sort of forced)

Longer-term I still feel like the picture is a bit unclear, however:

  • For the command-style _start this heavily depends on the signature of whatever the command entrypoint is. If it needs a post-return or a realloc it seems like the same problem all over again because how, at the component model level, does initialization happn before the realloc/etc are called?
  • For the reactor-style _init it's not clear to me who or what calls this _init function. I'm presuming there's an idea for a hypothetical component model feature to do this, though, hence the "unclear to me" feeling.

I know there's been a lot of historical discussion about command/reactor patterns in WASI but very little of that I feel has ended up getting surfaced into the component model itself. In that sense I find it difficult to think about commands/reactors since that's in theory a higher-level abstraction than the component model itself and the problem here in this issue is the component model's semantics. So "well it should be a reactor" or "let's fix commands" I find unclear since that's dealing with higher level semantics than the issue being run into here.

@alexcrichton
Copy link
Member

To link it @pchickey wrote up a great summary of how to tackle this at #99

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants