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

fix #17749 ignore SIGPIPE signals, fix nim CI #17748 #17752

Merged
merged 2 commits into from
Apr 18, 2021

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Apr 17, 2021

future work

  • add a lib/std/private/signals module to refactor posix signal specific code and avoid duplicating logic in several places (ansi_c, system/excpt, posix, segfaults, as well as simplify code eg: stdlib/tssl, rawsockets, blockSigpipe in lib/pure/net.nim
  • use sigaction instead of blockSigpipe, sigaction is deprecated and comes with many caveats, in particular for multithreaded programs
    https://stackoverflow.com/questions/231912/what-is-the-difference-between-sigaction-and-signal

Use sigaction() unless you've got very compelling reasons not to do so.

links

refs #9867 (comment)

Also I'd recommend that everyone running their Nim program as a SSL server should install SIG_IGN as the SIGPIPE handler (actually we might want to do this in the stdlib). SIGPIPE is a pretty old signal and is largely obsoleted by EPIPE that's returned from any failed send()/write() attempt on a closed socket/pipe (we don't get to handle this one because SIGPIPE got in the way).

@timotheecour timotheecour force-pushed the pr_signal_SIGPIPE_ignore branch from 070bbe5 to 438aa34 Compare April 17, 2021 08:42
@timotheecour timotheecour added the TODO: followup needed remove tag once fixed or tracked elsewhere label Apr 17, 2021
@timotheecour timotheecour force-pushed the pr_signal_SIGPIPE_ignore branch 2 times, most recently from 6a80af4 to a72cbe3 Compare April 17, 2021 19:20
@timotheecour timotheecour force-pushed the pr_signal_SIGPIPE_ignore branch from a72cbe3 to fb1c432 Compare April 17, 2021 19:21
@timotheecour timotheecour marked this pull request as ready for review April 17, 2021 21:36
@timotheecour timotheecour requested a review from Araq April 18, 2021 01:24
@timotheecour timotheecour changed the title fix #17749 ignore SIGPIPE signals fix #17749 ignore SIGPIPE signals, fix nim CI #17748 Apr 18, 2021
@Araq Araq merged commit 42c6eec into nim-lang:devel Apr 18, 2021
@ringabout ringabout mentioned this pull request Apr 18, 2021
@timotheecour timotheecour deleted the pr_signal_SIGPIPE_ignore branch April 18, 2021 16:30
PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: followup needed remove tag once fixed or tracked elsewhere
Projects
None yet
2 participants