Skip to content
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

[PLAT-7986] Ignore non-fatal SIGPIPEs #1295

Merged
merged 1 commit into from
Feb 16, 2022
Merged

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Feb 14, 2022

Goal

Stop reporting false SIGPIPE unhandled errors when ignored with SIG_IGN.

If an app uses sigignore(SIGPIPE) or signal(SIGPIPE, SIG_IGN) then when a SIGPIPE is raised, the process will not be terminated.

Bugsnag's hander assumes that the signal will be fatal and writes a crash report.

Changeset

Removes Bugsnag's signal handler for SIGPIPE if the previously configured handler was SIG_IGN.

Testing

Adds an E2E scenario that was able to reproduce the problem and verify the fix.

Updates SIGPIPEScenario to use a more realistic method to trigger a SIGPIPE.

@github-actions
Copy link

Bugsnag.framework binary size did not change - 912,288 bytes

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%     +56  +0.0%     +56    __TEXT,__text
  -0.2%     -56  -0.2%     -56    [__TEXT]
  [ = ]       0  [ = ]       0    TOTAL

Generated by 🚫 Danger

@nickdowell nickdowell force-pushed the nickdowell/sigpipe-ignore branch from 1575042 to 4cc4934 Compare February 14, 2022 15:52
@@ -212,6 +212,11 @@ bool bsg_kscrashsentry_installSignalHandler(
}
goto failed;
}
if (fatalSignals[i] == SIGPIPE &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if maybe we should check for SIG_IGN on all signals except KILL and STOP (which can't be ignored).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that might be worth a follow-up 👍

@nickdowell nickdowell merged commit 62da7c5 into next Feb 16, 2022
@nickdowell nickdowell deleted the nickdowell/sigpipe-ignore branch February 16, 2022 08:10
@nickdowell nickdowell changed the title Ignore non-fatal SIGPIPEs [PLAT-7986] Ignore non-fatal SIGPIPEs Feb 16, 2022
@nickdowell nickdowell mentioned this pull request Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants