From 94f8453d0a58d2ab20c3bbe7f687089dbdcb84af Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 6 May 2016 21:03:29 -0700 Subject: [PATCH] Bug fixes to error handling --- src/core/ext/client_config/client_channel.c | 4 ++-- src/core/ext/client_config/subchannel.c | 3 ++- .../lib/iomgr/socket_utils_common_posix.c | 4 ++-- src/core/lib/iomgr/tcp_server_posix.c | 2 +- test/core/util/port_server_client.c | 24 ++++++++++++------- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index 801cbe0702f..eaefa3be6d7 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -128,8 +128,8 @@ static void set_channel_connectivity_state_locked(grpc_exec_ctx *exec_ctx, /* mask= */ GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY, /* check= */ 0); } - grpc_connectivity_state_set(exec_ctx, &chand->state_tracker, state, error, - reason); + grpc_connectivity_state_set(exec_ctx, &chand->state_tracker, state, + GRPC_ERROR_REF(error), reason); } static void on_lb_policy_state_changed_locked(grpc_exec_ctx *exec_ctx, diff --git a/src/core/ext/client_config/subchannel.c b/src/core/ext/client_config/subchannel.c index 0f58dabb1ea..69b1fa5d343 100644 --- a/src/core/ext/client_config/subchannel.c +++ b/src/core/ext/client_config/subchannel.c @@ -484,7 +484,8 @@ static void subchannel_on_child_state_changed(grpc_exec_ctx *exec_ctx, void *p, sw->connectivity_state = GRPC_CHANNEL_FATAL_FAILURE; } grpc_connectivity_state_set(exec_ctx, &c->state_tracker, - sw->connectivity_state, error, "reflect_child"); + sw->connectivity_state, GRPC_ERROR_REF(error), + "reflect_child"); if (sw->connectivity_state != GRPC_CHANNEL_FATAL_FAILURE) { grpc_connected_subchannel_notify_on_state_change( exec_ctx, GET_CONNECTED_SUBCHANNEL(c, no_barrier), NULL, diff --git a/src/core/lib/iomgr/socket_utils_common_posix.c b/src/core/lib/iomgr/socket_utils_common_posix.c index 83941574df6..d1721c910c4 100644 --- a/src/core/lib/iomgr/socket_utils_common_posix.c +++ b/src/core/lib/iomgr/socket_utils_common_posix.c @@ -84,10 +84,10 @@ grpc_error *grpc_set_socket_no_sigpipe_if_possible(int fd) { if (0 != setsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &val, sizeof(val))) { return GRPC_OS_ERROR(errno, "setsockopt(SO_NOSIGPIPE)"); } - if (0 == getsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &newval, &intlen)) { + if (0 != getsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &newval, &intlen)) { return GRPC_OS_ERROR(errno, "getsockopt(SO_NOSIGPIPE)"); } - if ((newval != 0) == val) { + if ((newval != 0) != (val != 0)) { return GRPC_ERROR_CREATE("Failed to set SO_NOSIGPIPE"); } #endif diff --git a/src/core/lib/iomgr/tcp_server_posix.c b/src/core/lib/iomgr/tcp_server_posix.c index 3276cb4ba31..bb51ba2c763 100644 --- a/src/core/lib/iomgr/tcp_server_posix.c +++ b/src/core/lib/iomgr/tcp_server_posix.c @@ -308,7 +308,7 @@ static grpc_error *prepare_socket(int fd, const struct sockaddr *addr, return GRPC_ERROR_NONE; error: - GPR_ASSERT(err == GRPC_ERROR_NONE); + GPR_ASSERT(err != GRPC_ERROR_NONE); if (fd >= 0) { close(fd); } diff --git a/test/core/util/port_server_client.c b/test/core/util/port_server_client.c index a03dc0c83fe..8b8fc0983b6 100644 --- a/test/core/util/port_server_client.c +++ b/test/core/util/port_server_client.c @@ -67,7 +67,7 @@ static void freed_port_from_server(grpc_exec_ctx *exec_ctx, void *arg, freereq *pr = arg; gpr_mu_lock(pr->mu); pr->done = 1; - grpc_pollset_kick(pr->pollset, NULL); + GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(pr->pollset, NULL)); gpr_mu_unlock(pr->mu); } @@ -102,9 +102,13 @@ void grpc_free_port_using_server(char *server, int port) { gpr_mu_lock(pr.mu); while (!pr.done) { grpc_pollset_worker *worker = NULL; - grpc_pollset_work(&exec_ctx, pr.pollset, &worker, - gpr_now(GPR_CLOCK_MONOTONIC), - GRPC_TIMEOUT_SECONDS_TO_DEADLINE(1)); + if (!GRPC_LOG_IF_ERROR( + "pollset_work", + grpc_pollset_work(&exec_ctx, pr.pollset, &worker, + gpr_now(GPR_CLOCK_MONOTONIC), + GRPC_TIMEOUT_SECONDS_TO_DEADLINE(1)))) { + pr.done = 1; + } } gpr_mu_unlock(pr.mu); @@ -173,7 +177,7 @@ static void got_port_from_server(grpc_exec_ctx *exec_ctx, void *arg, GPR_ASSERT(port > 1024); gpr_mu_lock(pr->mu); pr->port = port; - grpc_pollset_kick(pr->pollset, NULL); + GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(pr->pollset, NULL)); gpr_mu_unlock(pr->mu); } @@ -208,9 +212,13 @@ int grpc_pick_port_using_server(char *server) { gpr_mu_lock(pr.mu); while (pr.port == -1) { grpc_pollset_worker *worker = NULL; - grpc_pollset_work(&exec_ctx, pr.pollset, &worker, - gpr_now(GPR_CLOCK_MONOTONIC), - GRPC_TIMEOUT_SECONDS_TO_DEADLINE(1)); + if (!GRPC_LOG_IF_ERROR( + "pollset_work", + grpc_pollset_work(&exec_ctx, pr.pollset, &worker, + gpr_now(GPR_CLOCK_MONOTONIC), + GRPC_TIMEOUT_SECONDS_TO_DEADLINE(1)))) { + pr.port = 0; + } } gpr_mu_unlock(pr.mu);