From 2da029647803aa26e393faa1422beecae7d1805a Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 6 May 2015 16:14:25 -0700 Subject: [PATCH] Eliminate need for SIGPIPE handling --- include/grpc/support/port_platform.h | 4 ++++ src/core/iomgr/socket_utils_common_posix.c | 13 +++++++++++++ src/core/iomgr/socket_utils_posix.h | 5 +++++ src/core/iomgr/tcp_client_posix.c | 3 ++- src/core/iomgr/tcp_posix.c | 8 +++++++- src/core/iomgr/tcp_server_posix.c | 5 ++++- test/core/util/test_config.c | 4 ---- test/cpp/qps/smoke_test.cc | 1 - 8 files changed, 35 insertions(+), 8 deletions(-) diff --git a/include/grpc/support/port_platform.h b/include/grpc/support/port_platform.h index 671648a976b..df7861c7b6d 100644 --- a/include/grpc/support/port_platform.h +++ b/include/grpc/support/port_platform.h @@ -80,6 +80,7 @@ #define GPR_POSIX_SYNC 1 #define GPR_POSIX_TIME 1 #define GPR_GETPID_IN_UNISTD_H 1 +#define GPR_HAVE_MSG_NOSIGNAL 1 #elif defined(__linux__) #ifndef _BSD_SOURCE #define _BSD_SOURCE @@ -124,6 +125,7 @@ #define GPR_POSIX_SYNC 1 #define GPR_POSIX_TIME 1 #define GPR_GETPID_IN_UNISTD_H 1 +#define GPR_HAVE_MSG_NOSIGNAL 1 #ifdef _LP64 #define GPR_ARCH_64 1 #else /* _LP64 */ @@ -155,6 +157,7 @@ #define GPR_POSIX_SYNC 1 #define GPR_POSIX_TIME 1 #define GPR_GETPID_IN_UNISTD_H 1 +#define GPR_HAVE_SO_NOSIGPIPE 1 #ifdef _LP64 #define GPR_ARCH_64 1 #else /* _LP64 */ @@ -180,6 +183,7 @@ #define GPR_POSIX_SYNC 1 #define GPR_POSIX_TIME 1 #define GPR_GETPID_IN_UNISTD_H 1 +#define GPR_HAVE_SO_NOSIGPIPE 1 #ifdef _LP64 #define GPR_ARCH_64 1 #else /* _LP64 */ diff --git a/src/core/iomgr/socket_utils_common_posix.c b/src/core/iomgr/socket_utils_common_posix.c index 3c8cafa3152..a9af5947009 100644 --- a/src/core/iomgr/socket_utils_common_posix.c +++ b/src/core/iomgr/socket_utils_common_posix.c @@ -76,6 +76,19 @@ int grpc_set_socket_nonblocking(int fd, int non_blocking) { return 1; } +int grpc_set_socket_no_sigpipe_if_possible(int fd) { +#ifdef GPR_HAVE_SO_NOSIGPIPE + int val = 1; + int newval; + socklen_t intlen = sizeof(newval); + return 0 == setsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &val, sizeof(val)) && + 0 == getsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &newval, &intlen) && + (newval != 0) == val; +#else + return 1; +#endif +} + /* set a socket to close on exec */ int grpc_set_socket_cloexec(int fd, int close_on_exec) { int oldflags = fcntl(fd, F_GETFD, 0); diff --git a/src/core/iomgr/socket_utils_posix.h b/src/core/iomgr/socket_utils_posix.h index c161082afc0..d2a315b4622 100644 --- a/src/core/iomgr/socket_utils_posix.h +++ b/src/core/iomgr/socket_utils_posix.h @@ -63,6 +63,11 @@ int grpc_set_socket_low_latency(int fd, int low_latency); state to library users, we turn off IPv6 sockets. */ int grpc_ipv6_loopback_available(void); +/* Tries to set SO_NOSIGPIPE if available on this platform. + Returns 1 on success, 0 on failure. + If SO_NO_SIGPIPE is not available, returns 1. */ +int grpc_set_socket_no_sigpipe_if_possible(int fd); + /* An enum to keep track of IPv4/IPv6 socket modes. Currently, this information is only used when a socket is first created, but diff --git a/src/core/iomgr/tcp_client_posix.c b/src/core/iomgr/tcp_client_posix.c index e20cc3d1b2e..2401fe00e45 100644 --- a/src/core/iomgr/tcp_client_posix.c +++ b/src/core/iomgr/tcp_client_posix.c @@ -69,7 +69,8 @@ static int prepare_socket(const struct sockaddr *addr, int fd) { } if (!grpc_set_socket_nonblocking(fd, 1) || !grpc_set_socket_cloexec(fd, 1) || - (addr->sa_family != AF_UNIX && !grpc_set_socket_low_latency(fd, 1))) { + (addr->sa_family != AF_UNIX && !grpc_set_socket_low_latency(fd, 1)) || + !grpc_set_socket_no_sigpipe_if_possible(fd)) { gpr_log(GPR_ERROR, "Unable to configure socket %d: %s", fd, strerror(errno)); goto error; diff --git a/src/core/iomgr/tcp_posix.c b/src/core/iomgr/tcp_posix.c index 06725fbc894..f7dae5f86ce 100644 --- a/src/core/iomgr/tcp_posix.c +++ b/src/core/iomgr/tcp_posix.c @@ -53,6 +53,12 @@ #include #include +#ifdef GPR_HAVE_MSG_NOSIGNAL +#define SENDMSG_FLAGS MSG_NOSIGNAL +#else +#define SENDMSG_FLAGS 0 +#endif + /* Holds a slice array and associated state. */ typedef struct grpc_tcp_slice_state { gpr_slice *slices; /* Array of slices */ @@ -461,7 +467,7 @@ static grpc_endpoint_write_status grpc_tcp_flush(grpc_tcp *tcp) { GRPC_TIMER_BEGIN(GRPC_PTAG_SENDMSG, 0); do { /* TODO(klempner): Cork if this is a partial write */ - sent_length = sendmsg(tcp->fd, &msg, 0); + sent_length = sendmsg(tcp->fd, &msg, SENDMSG_FLAGS); } while (sent_length < 0 && errno == EINTR); GRPC_TIMER_END(GRPC_PTAG_SENDMSG, 0); diff --git a/src/core/iomgr/tcp_server_posix.c b/src/core/iomgr/tcp_server_posix.c index 7e31f2d7a54..d1cd8a769cd 100644 --- a/src/core/iomgr/tcp_server_posix.c +++ b/src/core/iomgr/tcp_server_posix.c @@ -235,7 +235,8 @@ static int prepare_socket(int fd, const struct sockaddr *addr, int addr_len) { if (!grpc_set_socket_nonblocking(fd, 1) || !grpc_set_socket_cloexec(fd, 1) || (addr->sa_family != AF_UNIX && (!grpc_set_socket_low_latency(fd, 1) || - !grpc_set_socket_reuse_addr(fd, 1)))) { + !grpc_set_socket_reuse_addr(fd, 1))) || + !grpc_set_socket_no_sigpipe_if_possible(fd)) { gpr_log(GPR_ERROR, "Unable to configure socket %d: %s", fd, strerror(errno)); goto error; @@ -296,6 +297,8 @@ static void on_read(void *arg, int success) { } } + grpc_set_socket_no_sigpipe_if_possible(fd); + sp->server->cb( sp->server->cb_arg, grpc_tcp_create(grpc_fd_create(fd), GRPC_TCP_DEFAULT_READ_SLICE_SIZE)); diff --git a/test/core/util/test_config.c b/test/core/util/test_config.c index 1f0f0175b1c..be69fcf6750 100644 --- a/test/core/util/test_config.c +++ b/test/core/util/test_config.c @@ -49,10 +49,6 @@ static int seed(void) { return _getpid(); } #endif void grpc_test_init(int argc, char **argv) { -#ifndef GPR_WIN32 - /* disable SIGPIPE */ - signal(SIGPIPE, SIG_IGN); -#endif gpr_log(GPR_DEBUG, "test slowdown: machine=%f build=%f total=%f", GRPC_TEST_SLOWDOWN_MACHINE_FACTOR, GRPC_TEST_SLOWDOWN_BUILD_FACTOR, GRPC_TEST_SLOWDOWN_FACTOR); diff --git a/test/cpp/qps/smoke_test.cc b/test/cpp/qps/smoke_test.cc index 2c60a9997c8..1a448339409 100644 --- a/test/cpp/qps/smoke_test.cc +++ b/test/cpp/qps/smoke_test.cc @@ -138,7 +138,6 @@ static void RunQPS() { } // namespace grpc int main(int argc, char** argv) { - signal(SIGPIPE, SIG_IGN); using namespace grpc::testing; RunSynchronousStreamingPingPong(); RunSynchronousUnaryPingPong();