-
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
Consider adding support for MacOS exception ports #2456
Comments
Oh and yes, I found this because I added breakpad to our project so we can report crashes effectively, but then someone on mac had a panic in some of their WASM code, so I had to go down this rabbit hole. |
Thanks for the report! This seems like something that might be worthwhile to investigate, but somewhat orthogonally the signal handling in wasmtime should forward signals to the previous signal handler if any is registered. Did you find this to not work, however? If so could you describe a bit more about the bug that's happening with the signal handlers today? |
I think the problem is that the exception gets delivered to an exception port registered by the application before the wasmtime signal handler has the chance to run, thus causing all exceptions that should be handled by wasmtime to instead go to the exception port registered by the application. |
Yes, exactly what @bjorn3 said, I've worked around this by instead changing our breakpad fork to just not register the exception ports and instead hook all of the signals that the exception handler used to cover, which works nicely since it can seamlessly work with wasmtime, but it seems like having this in wasmtime would make sense so that it's not at the mercy of whatever other exception handler can be registered that doesn't play nicely. I was actually surprised that breakpad didn't have an option to change this, but I guess having both breakpad alongside a piece of code like wasmtime that depends on handling signals is fairly niche or something. |
Supporting this makes sense to me. Probably faster too, not that it really matters in this particular case 😆 |
Ah yes thanks for clarifying! Also, yes, agreed we should add this to Wasmtime! Just need to figure out how... |
The old SignalHandlers.cpp file removed in #1365 theoretically had code to do this, though the code is in C++; it may help show what needs to be done here. |
I was poking around this a bit last night (here's the file in its full glory), and one snag is going to be that the exception handler runs on a separate thread because a different thread receives the mach message. Our determination of whether a wasm trap ocurred, however, requires access to thread-local state (e.g. to read the store's map of registered JIT addresses and see if we collide with any of them). If we're running the handler on a separate thread, though, I'm not sure how the mach exception handling thread will know how to associated a store's JIT data with the exception that it just received. One possible solution is to have a global map that's updated on entry/exit into wasm, but that would require mutex-like synchronization which could be somewhat expensive too. |
Looking at SpiderMonkey, it appears that registers code segments in a process-global map which has synchronized access. I think the answer for wasmtime is likely the same. Somehow we'll need to have cheap-ish "is this a jit region" test and then some way to go from the register state to the thread-local information about the |
This commit moves macOS to using mach ports instead of signals for handling traps. The motivation for this is listed in bytecodealliance#2456, namely that once mach ports are used in a process that means traditional UNIX signal handlers won't get used. This means that if Wasmtime is integrated with Breakpad, for example, then Wasmtime's trap handler never fires and traps don't work. The `traphandlers` module is refactored as part of this commit to split the platform-specific bits into their own files (it was growing quite a lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files remain the same as they were before with a few minor tweaks for some refactored interfaces. The `macos.rs` file is brand new and lifts almost its entire implementation from SpiderMonkey, adapted for Wasmtime though. The main gotcha with mach ports is that a separate thread is what services the exception. Some unsafe magic allows this separate thread to read non-`Send` and temporary state from other threads, but is hoped to be safe in this context. The unfortunate downside is that calling wasm on macOS now involves taking a global lock and modifying a global hash map twice-per-call. I'm not entirely sure how to get out of this cost for now, but hopefully for any embeddings on macOS it's not the end of the world. Closes bytecodealliance#2456
This commit moves macOS to using mach ports instead of signals for handling traps. The motivation for this is listed in bytecodealliance#2456, namely that once mach ports are used in a process that means traditional UNIX signal handlers won't get used. This means that if Wasmtime is integrated with Breakpad, for example, then Wasmtime's trap handler never fires and traps don't work. The `traphandlers` module is refactored as part of this commit to split the platform-specific bits into their own files (it was growing quite a lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files remain the same as they were before with a few minor tweaks for some refactored interfaces. The `macos.rs` file is brand new and lifts almost its entire implementation from SpiderMonkey, adapted for Wasmtime though. The main gotcha with mach ports is that a separate thread is what services the exception. Some unsafe magic allows this separate thread to read non-`Send` and temporary state from other threads, but is hoped to be safe in this context. The unfortunate downside is that calling wasm on macOS now involves taking a global lock and modifying a global hash map twice-per-call. I'm not entirely sure how to get out of this cost for now, but hopefully for any embeddings on macOS it's not the end of the world. Closes bytecodealliance#2456
This commit moves macOS to using mach ports instead of signals for handling traps. The motivation for this is listed in bytecodealliance#2456, namely that once mach ports are used in a process that means traditional UNIX signal handlers won't get used. This means that if Wasmtime is integrated with Breakpad, for example, then Wasmtime's trap handler never fires and traps don't work. The `traphandlers` module is refactored as part of this commit to split the platform-specific bits into their own files (it was growing quite a lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files remain the same as they were before with a few minor tweaks for some refactored interfaces. The `macos.rs` file is brand new and lifts almost its entire implementation from SpiderMonkey, adapted for Wasmtime though. The main gotcha with mach ports is that a separate thread is what services the exception. Some unsafe magic allows this separate thread to read non-`Send` and temporary state from other threads, but is hoped to be safe in this context. The unfortunate downside is that calling wasm on macOS now involves taking a global lock and modifying a global hash map twice-per-call. I'm not entirely sure how to get out of this cost for now, but hopefully for any embeddings on macOS it's not the end of the world. Closes bytecodealliance#2456
I've got a prototype of this at #2632 now. @Jake-Shadle if you're still interested mind giving that a spin and seeing if it works for your use case? |
We'd be happy to try this out, though I probably can't get to it this week, but next week! |
This commit moves macOS to using mach ports instead of signals for handling traps. The motivation for this is listed in bytecodealliance#2456, namely that once mach ports are used in a process that means traditional UNIX signal handlers won't get used. This means that if Wasmtime is integrated with Breakpad, for example, then Wasmtime's trap handler never fires and traps don't work. The `traphandlers` module is refactored as part of this commit to split the platform-specific bits into their own files (it was growing quite a lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files remain the same as they were before with a few minor tweaks for some refactored interfaces. The `macos.rs` file is brand new and lifts almost its entire implementation from SpiderMonkey, adapted for Wasmtime though. The main gotcha with mach ports is that a separate thread is what services the exception. Some unsafe magic allows this separate thread to read non-`Send` and temporary state from other threads, but is hoped to be safe in this context. The unfortunate downside is that calling wasm on macOS now involves taking a global lock and modifying a global hash map twice-per-call. I'm not entirely sure how to get out of this cost for now, but hopefully for any embeddings on macOS it's not the end of the world. Closes bytecodealliance#2456
This commit moves macOS to using mach ports instead of signals for handling traps. The motivation for this is listed in bytecodealliance#2456, namely that once mach ports are used in a process that means traditional UNIX signal handlers won't get used. This means that if Wasmtime is integrated with Breakpad, for example, then Wasmtime's trap handler never fires and traps don't work. The `traphandlers` module is refactored as part of this commit to split the platform-specific bits into their own files (it was growing quite a lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files remain the same as they were before with a few minor tweaks for some refactored interfaces. The `macos.rs` file is brand new and lifts almost its entire implementation from SpiderMonkey, adapted for Wasmtime though. The main gotcha with mach ports is that a separate thread is what services the exception. Some unsafe magic allows this separate thread to read non-`Send` and temporary state from other threads, but is hoped to be safe in this context. The unfortunate downside is that calling wasm on macOS now involves taking a global lock and modifying a global hash map twice-per-call. I'm not entirely sure how to get out of this cost for now, but hopefully for any embeddings on macOS it's not the end of the world. Closes bytecodealliance#2456
This commit moves macOS to using mach ports instead of signals for handling traps. The motivation for this is listed in bytecodealliance#2456, namely that once mach ports are used in a process that means traditional UNIX signal handlers won't get used. This means that if Wasmtime is integrated with Breakpad, for example, then Wasmtime's trap handler never fires and traps don't work. The `traphandlers` module is refactored as part of this commit to split the platform-specific bits into their own files (it was growing quite a lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files remain the same as they were before with a few minor tweaks for some refactored interfaces. The `macos.rs` file is brand new and lifts almost its entire implementation from SpiderMonkey, adapted for Wasmtime though. The main gotcha with mach ports is that a separate thread is what services the exception. Some unsafe magic allows this separate thread to read non-`Send` and temporary state from other threads, but is hoped to be safe in this context. The unfortunate downside is that calling wasm on macOS now involves taking a global lock and modifying a global hash map twice-per-call. I'm not entirely sure how to get out of this cost for now, but hopefully for any embeddings on macOS it's not the end of the world. Closes bytecodealliance#2456
This commit moves macOS to using mach ports instead of signals for handling traps. The motivation for this is listed in bytecodealliance#2456, namely that once mach ports are used in a process that means traditional UNIX signal handlers won't get used. This means that if Wasmtime is integrated with Breakpad, for example, then Wasmtime's trap handler never fires and traps don't work. The `traphandlers` module is refactored as part of this commit to split the platform-specific bits into their own files (it was growing quite a lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files remain the same as they were before with a few minor tweaks for some refactored interfaces. The `macos.rs` file is brand new and lifts almost its entire implementation from SpiderMonkey, adapted for Wasmtime though. The main gotcha with mach ports is that a separate thread is what services the exception. Some unsafe magic allows this separate thread to read non-`Send` and temporary state from other threads, but is hoped to be safe in this context. The unfortunate downside is that calling wasm on macOS now involves taking a global lock and modifying a global hash map twice-per-call. I'm not entirely sure how to get out of this cost for now, but hopefully for any embeddings on macOS it's not the end of the world. Closes bytecodealliance#2456
Currently, wasmtime hooks various signals in traphandlers on Unix platforms before executing wasm code so that it can catch and handle various runtime issues without actually crashing the host program. However, on MacOS there is an additional error handling mechanism, Mach Exceptions. By default a Mach exception will be turned into a unix signal, however, if the application binds their own exception port via
task_set_exception_ports
or similar, it's up to that handler to actual forward to the default signal mechanism, but in most cases they will probably just abort the process instead, even though wasmtime would have successfully recovered program execution in its signal handler.Feature
The feature I'm proposing would be for wasmtime's traphandler to use exception ports when targeting MacOS (and I guess iOS, but I would assume that target is not relevant for wasmtime due to its code execution rules?), forwarding exceptions to whatever previously registered exception port if the exception is not one of the one's wasmtime wants to handle.
Benefit
The application could have previously registered signal handlers or exception ports on Mac and still function as expected whenever wasmtime executes exceptional wasm code.
Implementation
* I think basically the implementation is mostly conceptually the same as the current signal handling approach, but basically it would be something like...
Also, one thing that would possibly make this easier would just be to directly signal the thread, similar to Mac's own ux_handler code as seen here.
* All of this information is gleaned from Mach Exceptions, Signas, and Breakpad's Exception Handler, as the official Apple documentation is, frankly, garbage.
Alternatives
The alternative would just be to not do this at all, but in that case I think having a warning in the documentation somewhere so that people running wasmtime on a Mac are aware that having
task*_set_exception_ports
in their program might mean that wasmtime's normal behavior of catching and handling exception wasm code won't even execute depending on how the user exception handler is implemented.The text was updated successfully, but these errors were encountered: