Signals are still reported as error even if they are ignored

[alokedesai]

Environment

How do you use Sentry?
Sentry SaaS (sentry.io)

Which SDK and version?
sentry.cocoa 7.0.3

Steps to Reproduce

1. Explicitly ignore SIGIPE: signal(SIGPIPE, SIG_IGN)
2. Send SIGPIPE to the process (kill -SIGPIPE {pid})

Expected Result

No crash in sentry--the signal was explicitly ignored by Sentry.

Actual Result

A fatal crash in Sentry--I think this because Sentry sets a custom signal handler but doesn't check if the previous handler is SIG_IGN first:

sentry-cocoa/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_Signal.c

Line 165 in c06c857

if (sigaction(fatalSignals[i], &action, &g_previousSignalHandlers[i]) != 0) {

[philipphofmann]

Thanks for reporting this issue, @alokedesai. Would you please elaborate on why you want to ignore SIGIPE explicitly?

[alokedesai]

I work at warp.dev--our app is written in Rust with a very thin layer of Objective C. Rust by default ignores Sigpipe (see rust-lang/rust#13158), so it's not even something we're configuring directly, it's more a function of our stack. Generally, my understanding is that you should be ignoring SIGPIPE unless you're writing a CLI app since you don't want a broken pipe to terminate your app (see here as an example).

In practice this means that sending a SIGPIPE to our application does nothing (since SIGPIPE is ignored) but does cause Sentry to report an "unhanded", fatal Sigpipe crash even though the app ignored the signal and continued to run just fine.

[philipphofmann]

Thanks, @alokedesai, for the detailed explanation.

@Swatinem, you know more about signal handlers than me. Does this change make sense to you?

[Swatinem]

I think thats a reasonable thing to do (check the previous signal handler, and essentially forward the ignore)

[philipphofmann]

We will put this on our backlog @alokedesai. This seems like a quick win, but we can't give you an ETA at the moment. Of course, we are happy to accept PRs 😀.

[alokedesai]

I'm happy to send a PR for this, @Swatinem though just to confirm we're on the same page--it's not that we need to forward the ignore, it's that we should check if the existing handler is SIG_IGN and, if so, not intercept the signal or try to send a crash.

[Swatinem]

Ah okay, maybe there was a bit of confusion. Checking the current handler and skipping registering our own would be good as well.

[alokedesai]

Just sent out a very simple diff for this #1489. Would love ideas/links on how to add tests for this since it requires a real executable for correct signal handling.