Stream: git-wasmtime

Topic: wasmtime / issue #3052 unix::StoreExt no longer available...

Wasmtime GitHub notifications bot (Jul 02 2021 at 11:23):

ulan labeled issue #3052:

We are using unix::StoreExt::set_signal_handler() to install a custom signal handler. That no longer compiles on MacOS after PR #2632.

If I understand correctly, trap handling works internally in wasmtime, but is no longer exposed via the unix::StoreExt API. Is it due to some fundamental restriction of mach ports or could it be implemented?

cc @alexcrichton, @bnjbvr

Wasmtime GitHub notifications bot (Jul 02 2021 at 11:23):

ulan opened issue #3052:

We are using unix::StoreExt::set_signal_handler() to install a custom signal handler. That no longer compiles on MacOS after PR #2632.

If I understand correctly, trap handling works internally in wasmtime, but is no longer exposed via the unix::StoreExt API. Is it due to some fundamental restriction of mach ports or could it be implemented?

cc @alexcrichton, @bnjbvr

Wasmtime GitHub notifications bot (Jul 02 2021 at 12:10):

ulan commented on issue #3052:

A workaround would be to introduce a cargo feature flag to select between signals and mach ports on MacOS. Would you accept such a patch for upstreaming?

Wasmtime GitHub notifications bot (Jul 02 2021 at 12:37):

bjorn3 commented on issue #3052:

Why do you want to use signals on macOS? The switch happened to prevent conflicts with crash reporters that use mach ports and thus would get a benign SIGSEGV before the wasmtime signal handler could handle it.

Wasmtime GitHub notifications bot (Jul 02 2021 at 14:01):

alexcrichton commented on issue #3052:

Mach ports are such a different mechanism from signals that I ended up removing the signal-handler-registration code for macOS since it didn't really make sense any more. Mach ports catch exceptions on a separate thread while signals always run on the same thread.

That, coupled with the fact that you can always install and manage your own signal handler, is why I ended up removing this from macOS without replacement. It's always been the case that even for Unix you can install your own signal handler as well.

Could you describe your use case a bit more? Does your use case require Wasmtime to handle traps first and then your application's handler runs next? Can your application register its own signal handler and does that work?

Wasmtime GitHub notifications bot (Jul 02 2021 at 15:35):

ulan commented on issue #3052:

Thanks @bjorn3 and @alexcrichton! The motivation for PR #2632 makes a lot of sense to me.

Our use case is the motivation behind PR #620: we use a custom signal handler to keep track of accessed Wasm memory pages. The handler runs before the Wasmtime handler, but we need to run the Wasmtime handler afterwards. What makes it more complex is that there are multiple handlers (one per Wasm instance) and multiple Wasm instances can be running on multiple threads.

Registering our own signal handler is probably possible in theory, but it is very difficult to get the handler chaining, registration ordering, and Wasm instance tracking per thread right. We would need to mirror what Wasmtime already does internally.

It would be great if we could find a workaround to unblock updating to the current version of Wasmtime. It's worth mentioning that we don't use Breakpad, so the signal-based trap handling would work well for us on MacOS.

Wasmtime GitHub notifications bot (Jul 02 2021 at 15:40):

alexcrichton commented on issue #3052:

Oh oops sorry I was aware of that original use case but didn't realize y'all were working with it on macOS as well, sorry about that!

I think that having a compile-time feature for using signal handlers instead of mach exceptions makes sense to me, afaik it should be a pretty small switch. We can then conditionally define the StoreExt trait on macOS depending on this feature being enabled.

If you're up for sending a PR, that'd be great!

Wasmtime GitHub notifications bot (Jul 02 2021 at 15:46):

ulan commented on issue #3052:

Thanks a lot @alexcrichton! And no worries at all about the breakage. I'll try to get the PR ready :)

Wasmtime GitHub notifications bot (Jul 13 2021 at 11:16):

ulan commented on issue #3052:

Thanks for merging the fix @alexcrichton! Would it be possible to cherry-pick the fix to the stable branch stable-v0.26 and release it?

FWIW, I prepared PR #3079 that applies cleanly to that branch and passes our tests locally.

Wasmtime GitHub notifications bot (Jul 14 2021 at 07:34):

ulan commented on issue #3052:

@alexcrichton and I discussed backporting of the fix to 0.26 in PR #3079.

The summary:

• Currently only security fixes are backported.
• Newer versions wasmtime are as suitable for production as 0.26.
• Instead of backporting, it is better to update to the current main of wasmtime.

I will start working on updating and will also investigate if it is possible to float the patch on our end to make updating more incremental.

Closing the issue as fixed. Thanks all for helping!

Wasmtime GitHub notifications bot (Jul 14 2021 at 07:34):

ulan closed issue #3052:

We are using unix::StoreExt::set_signal_handler() to install a custom signal handler. That no longer compiles on MacOS after PR #2632.

If I understand correctly, trap handling works internally in wasmtime, but is no longer exposed via the unix::StoreExt API. Is it due to some fundamental restriction of mach ports or could it be implemented?

cc @alexcrichton, @bnjbvr

Last updated: Nov 22 2024 at 16:03 UTC