Stream: git-wasmtime

Topic: wasmtime / PR #3063 Restore POSIX signal handling on MacO...


view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:31):

ulan opened PR #3063 from posix-signals-on-macos to main:

As described in Issue #3052, the switch to Mach Exception handling
removed unix::StoreExt from the public API of crate on MacOS.
That is a breaking change and makes it difficult for some
application to upgrade to the current stable Wasmtime.

As a workaround this PR introduces a feature flag called
posix-signals-on-macos that when enabled restores the old behaviour
on MacOS. The flag is disabled by default.

The only non-trivial changes in this PR are in traphandlers/unix.rs that
restore MacOS-related code removed from traphandlers.rs in PR #2723.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:37):

ulan has marked PR #3063 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:37):

ulan submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:37):

ulan submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:37):

ulan created PR review comment:

Not sure about the name. Bikeshedding is welcome. Possible alternatives: macos-signals, macos-posix-signals.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:37):

ulan created PR review comment:

This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L205 without any changes.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:37):

ulan created PR review comment:

This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L223 without any changes.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:37):

ulan created PR review comment:

This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L124 after renaming Unwind to wasmtime_longjmp. The comment is also adjusted accordingly.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 16:59):

ulan updated PR #3063 from posix-signals-on-macos to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 05 2021 at 17:07):

ulan edited PR #3063 from posix-signals-on-macos to main:

As described in Issue #3052, the switch to Mach Exception handling
removed unix::StoreExt from the public API of the crate on MacOS.
That is a breaking change and makes it difficult for some
application to upgrade to the current stable Wasmtime.

As a workaround this PR introduces a feature flag called
posix-signals-on-macos that when enabled restores the old behaviour
on MacOS. The flag is disabled by default.

The only non-trivial changes in this PR are in traphandlers/unix.rs that
restore MacOS-related code removed from traphandlers.rs in PR #2723.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jul 12 2021 at 14:05):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 12 2021 at 14:05):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 12 2021 at 14:05):

alexcrichton created PR review comment:

I think it's probably fine to skip this feature on the wasmtime-cli crate since it's mostly the wasmtime embedding crate that wants this.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 12 2021 at 20:10):

ulan submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 12 2021 at 20:10):

ulan created PR review comment:

It is useful for conditionally enabling tests/all/custom_signal_handler.rs on MacOS and running the tests with cargo test --features=posix-signals-on-macos custom_signal_handler
But indeed it's not needed for production. Should I remove it?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 12 2021 at 20:12):

ulan updated PR #3063 from posix-signals-on-macos to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 12 2021 at 21:25):

alexcrichton merged PR #3063.


Last updated: Dec 23 2024 at 12:05 UTC