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

SIGPIPE handling issue? #2

Open
keigoi opened this issue Sep 24, 2021 · 2 comments
Open

SIGPIPE handling issue? #2

keigoi opened this issue Sep 24, 2021 · 2 comments

Comments

@keigoi
Copy link

keigoi commented Sep 24, 2021

Hi! in the code below, the SIGPIPE handler is set to avoid write failures, but I see this is not really good practice for servers (see https://stackoverflow.com/a/7777789/515485 for example).
This part would intercept/change the original signal behaviour of the caller.

https://github.com/geneanet/ocaml-syslog/blob/v2.0.2/syslog.ml#L181-L195

It seems better to ignore SIGPIPE, or just leave the SIGPIPE handler default as-is.

However, it would be a breaking change in anyways for some users, so I'd like to just leave a note for now.

Actually, I had experienced a very subtle issue in the past, where the accidental server socket shutdown invoked this signal handler, and the socket fd was reused by the ocaml-syslog due to the handler behaviour above. The result is that my TCP packets to the server are redirected to the syslogd :p I forgot this phenomenon for years, and suddenly remembered it for now and writing this.

Cheers,

@sagotch
Copy link
Contributor

sagotch commented Sep 24, 2021

Shouldn't fallback just restore the old handler after trying to reconnect?

If no thread is involved, this should not "intercept/change the original signal behavior of the caller". If thread is involved, it might "intercept/change the original signal behavior of the caller" indeed, as it already does now. One could imagine a compile-time option to enable or not the SIGPIPE handling.

@keigoi
Copy link
Author

keigoi commented Sep 24, 2021

After some research, a standard solution seems to be just setting SIGPIPE to SIG_IGN (ignoring it) rather than handling it explicitly (problem above occurs) or leave it as default (process terminates).
Then, the write call on the disconnected syslogd socket will raise the exception (EPIPE) and we could properly handle it in the exception handler.

Indeed it doesn't affect a single-threaded caller.

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

No branches or pull requests

2 participants