Stream: git-wasmtime

Topic: wasmtime / issue #6762 c-api: Expose async_stack_size con...

view this post on Zulip Wasmtime GitHub notifications bot (Jul 24 2023 at 18:44):

github-actions[bot] commented on issue #6762:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Jul 24 2023 at 18:46):

alexcrichton commented on issue #6762:

Thanks! Can you elaborate a bit more on how this is used though? Async isn't exposed through the C API so in theory this configuration option shouldn't do anything in the C API, but if it's causing issues then that's a bug where when async is turned off it shouldn't be consulted.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 24 2023 at 18:55):

guregu commented on issue #6762:

@alexcrichton I agree that's it's more likely a bug somewhere else.

Basically, if I try to set the stack size to 8MB in wasmtime-go it will fail with this message:

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: max_wasm_stack size cannot exceed the async_stack_size', crates/c-api/src/engine.rs:33:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
SIGABRT: abort
PC=0x198cf4724 m=0 sigcode=0
signal arrived during cgo execution

So I figured exposing this knob could work around that. But perhaps the issue lies elsewhere :thinking:

view this post on Zulip Wasmtime GitHub notifications bot (Jul 24 2023 at 18:58):

guregu edited a comment on issue #6762:

@alexcrichton I agree that's it's more likely a bug (or maybe misconfiguration?) somewhere else.

Basically, if I try to set the stack size to 8MB in wasmtime-go it will fail with this message:

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: max_wasm_stack size cannot exceed the async_stack_size', crates/c-api/src/engine.rs:33:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
SIGABRT: abort
PC=0x198cf4724 m=0 sigcode=0
signal arrived during cgo execution

So I figured exposing this knob could work around that. But perhaps the issue lies elsewhere :thinking:

Would be happy to move this to wasmtime-go issue if that is the case!

view this post on Zulip Wasmtime GitHub notifications bot (Jul 24 2023 at 19:05):

guregu commented on issue #6762:

@alexcrichton I made an issue here for the wasmtime-go problem: https://github.com/bytecodealliance/wasmtime-go/issues/182. Please feel free to close this if it's not needed after all!

view this post on Zulip Wasmtime GitHub notifications bot (Jul 24 2023 at 19:32):

alexcrichton commented on issue #6762:

Oh no you're all good, I think that this is still a bug in Wasmtime, so this is the right place to fix it. I don't think there's a way to work around that from wasmtime-go right now.

I think the fix though is to update this condition to additionally check async_support because if that's disabled then there's no need to test anything about the async stack size, which I think would fix your use case because async_support is false for the C API.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 24 2023 at 19:46):

guregu commented on issue #6762:

Aha, that makes sense! I will take that approach in a new PR. Thank you for the help :pray:

view this post on Zulip Wasmtime GitHub notifications bot (Jul 24 2023 at 19:53):

jameysharp commented on issue #6762:

I think that's a good fix to apply. In addition it occurs to me that perhaps the C API should build the wasmtime crate with the async feature disabled, since it can't make use of the async support anyway. That would have avoided this bug because the if statement in question wouldn't have been compiled in at all.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 25 2023 at 16:19):

guregu commented on issue #6762:

New PR that takes the suggested approach: https://github.com/bytecodealliance/wasmtime/pull/6771

Last updated: Dec 23 2024 at 12:05 UTC