-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
c-api: Expose async_stack_size configuration #6762
Conversation
Subscribe to Label Actioncc @peterhuene
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 |
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. |
@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:
So I figured exposing this knob could work around that. But perhaps the issue lies elsewhere 🤔 Would be happy to move this to wasmtime-go issue if that is the case! |
@alexcrichton I made an issue here for the wasmtime-go problem: bytecodealliance/wasmtime-go#182. Please feel free to close this if it's not needed after all! |
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 |
Aha, that makes sense! I will take that approach in a new PR. Thank you for the help 🙏 |
I think that's a good fix to apply. In addition it occurs to me that perhaps the C API should build the |
Fixes an issue where max_wasm_stack was being validated against async_stack_size when async_support was disabled, discussed in bytecodealliance#6762.
New PR that takes the suggested approach: #6771 |
Fixes an issue where max_wasm_stack was being validated against async_stack_size when async_support was disabled, discussed in #6762.
Adds wasm_config_async_stack_size_set to C API. Also updates the stack-related comments in config.h to match the latest Rust ones.
Motivation: I want to set the stack size in wasmtime-go, but max_wasm_stack is limited by async_stack_size, so we need to set this too.