-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
macos: restoring signal disposition doesn't work as flags are not propagated #4299
Comments
There appear to be some mistakes in that commit, which don't entirely negate the fact this appears to be a kernel bug, but is making it work poorly. In particular
looks like it is missing a conditional set:
since libuv may re-register its own sigaction to change from SA_RESETHAND => 0, and that would overwrite the correct info for the old sigaction, leading to libuv not correctly unregistering itself later |
This reverts commit b9421d7. Refs: libuv#4299 Refs: libuv#4248
@slavamuravey fyi
Yes... Guess there isn't much we can do here except maybe add a regression test to ensure we don't forget this tidbit of macos lore and merge another attempt in the future. |
@bnoordhuis @vtjnash @santigimeno
|
I don't think that would work reliably. A third party (like another library) may have changed the signal handler before libuv gets to it. Longstanding libuv philosophy is that features should either work always or never, not sometimes. Users can code defensively around the first two but not the last one. |
@bnoordhuis Ok, then we should wait for changes in macos. After macos behavior will be the same as in linux for our case, we will be able to merge reverted PR again? |
Yes, but that may be a while because with macos we generally support the latest-1 or latest-2 releases (e.g. macos 11 and up right now.) |
Got it, thanks! |
@bnoordhuis What do you think, what if we apply changes from reverted PR for everything except of macos? And when issues will be fixed in macos, we apply changes for it too. |
Libuv's goal is to abstract away platform differences. If something can't be made to work in a cross-platform way, we don't do it. macOS (and Linux and Windows) are popular enough that we can't just shrug and say "too bad, doesn't work here." |
Take this very basic example:
Running this on linux produces the correct output:
Whereas on macos (arm64) it doesn't:
I observed this while running current
v1.x
with the node.js test suite and caused somesignal
tests to fail. They started failing after b9421d7, though I don't know if there's much we can do from the libuv side as it seems a macos issue. Thoughts?The text was updated successfully, but these errors were encountered: