Adar Dembo has posted comments on this change. Change subject: Ignore SIGPIPE earlier in startup process ......................................................................
Patch Set 1: (3 comments) > > What's the plan for the client? > > I thought we could deal with the client in a separate patch. The > idea I had there was to add an option to KuduClientBuilder called > ignore_sigpipe(bool) that defaults to true. If clients want to > manage their own signal handling themselves, at a process- or > thread- level, then they can set it to false when constructing the > client. If it's set to true, it calls something like > EnsureSigPipeIgnored(). I don't think we even have to go that far; documenting that it's the application's responsibility to ignore SIGPIPE before setting up a TLS-enabled KuduClient should suffice. Unlike ignore_sigpipe(bool), this can be removed in the future once we do the BIO wrapper thing (mentioned in a different comment). We should also find out whether Impala is vulnerable to this. Like us, squeasel's sq_start() will configure SIGPIPE to SIG_IGN, but also like us, it's possible that they may have called send() on TLS sockets by then. http://gerrit.cloudera.org:8080/#/c/6262/1/src/kudu/util/logging.cc File src/kudu/util/logging.cc: Line 269: // Ignore SIGPIPE early in the startup process so that threads writing to TLS Can you file a JIRA about this? Last night Todd mentioned that we can implement a BIO wrapper for openssl that'll let us explicitly set MSG_NOSIGNAL in send() calls. Once that happens, I'd argue we should remove this code. http://gerrit.cloudera.org:8080/#/c/6262/1/src/kudu/util/logging.h File src/kudu/util/logging.h: PS1, Line 262: ignore Nit: SIG_IGN, to be more precise. http://gerrit.cloudera.org:8080/#/c/6262/1/src/kudu/util/subprocess.cc File src/kudu/util/subprocess.cc: Line 76: typedef sighandler_t SignalHandlerCallback; Is this still needed? -- To view, visit http://gerrit.cloudera.org:8080/6262 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I040bd38ff31451ed9e25e7cf2127c869cf08a628 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
