aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Sree Kuchibhotla <sreek@google.com>2016-06-13 16:43:14 -0700
committerGravatar Sree Kuchibhotla <sreek@google.com>2016-06-13 16:43:14 -0700
commit41622a8e389e8eda38d6d3bfbf34cbf35f437156 (patch)
tree5c7f092926cf06fe233deb6526dbe1b10fcb923e
parent58e589644403b10afb31ffd45befabe13b652db8 (diff)
Fix tsan failures
-rw-r--r--Makefile1
-rw-r--r--build.yaml1
-rw-r--r--src/core/lib/iomgr/ev_epoll_linux.c27
3 files changed, 28 insertions, 1 deletions
diff --git a/Makefile b/Makefile
index 4d8b060760..28d6842c76 100644
--- a/Makefile
+++ b/Makefile
@@ -200,6 +200,7 @@ LD_tsan = clang
LDXX_tsan = clang++
CPPFLAGS_tsan = -O0 -fsanitize=thread -fno-omit-frame-pointer -Wno-unused-command-line-argument -DGPR_NO_DIRECT_SYSCALLS
LDFLAGS_tsan = -fsanitize=thread
+DEFINES_tsan = _GRPC_TSAN
DEFINES_tsan += GRPC_TEST_SLOWDOWN_BUILD_FACTOR=5
VALID_CONFIG_stapprof = 1
diff --git a/build.yaml b/build.yaml
index 85b66d985b..139ab3e8bc 100644
--- a/build.yaml
+++ b/build.yaml
@@ -3231,6 +3231,7 @@ configs:
CPPFLAGS: -O0 -fsanitize=thread -fno-omit-frame-pointer -Wno-unused-command-line-argument
-DGPR_NO_DIRECT_SYSCALLS
CXX: clang++
+ DEFINES: _GRPC_TSAN
LD: clang
LDFLAGS: -fsanitize=thread
LDXX: clang++
diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c
index a8a874cd4b..35a15e00c9 100644
--- a/src/core/lib/iomgr/ev_epoll_linux.c
+++ b/src/core/lib/iomgr/ev_epoll_linux.c
@@ -236,6 +236,17 @@ static grpc_wakeup_fd polling_island_wakeup_fd;
static gpr_mu g_pi_freelist_mu;
static polling_island *g_pi_freelist = NULL;
+#ifdef _GRPC_TSAN
+/* Currently TSAN may incorrectly flag data races between epoll_ctl and
+ epoll_wait for any grpc_fd structs that are added to the epoll set via
+ epoll_ctl and are returned (within a very short window) via epoll_wait().
+
+ To work-around this race, we establish a happens-before relation between
+ the code just-before epoll_ctl() and the code after epoll_wait() by using
+ this atomic */
+gpr_atm g_epoll_sync;
+#endif
+
/* The caller is expected to hold pi->mu lock before calling this function */
static void polling_island_add_fds_locked(polling_island *pi, grpc_fd **fds,
size_t fd_count, bool add_fd_refs) {
@@ -243,6 +254,11 @@ static void polling_island_add_fds_locked(polling_island *pi, grpc_fd **fds,
size_t i;
struct epoll_event ev;
+#ifdef _GRPC_TSAN
+ /* See the definition of g_epoll_sync for more context */
+ gpr_atm_rel_store(&g_epoll_sync, 0);
+#endif
+
for (i = 0; i < fd_count; i++) {
ev.events = (uint32_t)(EPOLLIN | EPOLLOUT | EPOLLET);
ev.data.ptr = fds[i];
@@ -361,6 +377,7 @@ static polling_island *polling_island_create(grpc_fd *initial_fd,
}
pi->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
+
if (pi->epoll_fd < 0) {
gpr_log(GPR_ERROR, "epoll_create1() failed with error: %s",
strerror(errno));
@@ -1144,6 +1161,11 @@ static void pollset_work_and_unlock(grpc_exec_ctx *exec_ctx,
}
}
+#ifdef _GRPC_TSAN
+ /* See the definition of g_poll_sync for more details */
+ gpr_atm_acq_load(&g_epoll_sync);
+#endif
+
for (int i = 0; i < ep_rv; ++i) {
void *data_ptr = ep_ev[i].data.ptr;
if (data_ptr == &grpc_global_wakeup_fd) {
@@ -1514,10 +1536,13 @@ const grpc_event_engine_vtable *grpc_init_epoll_linux(void) {
return &vtable;
}
-#else /* defined(GPR_LINUX_EPOLL) */
+#else /* defined(GPR_LINUX_EPOLL) */
+#if defined(GPR_POSIX_SOCKET)
+#include "src/core/lib/iomgr/ev_posix.h"
/* If GPR_LINUX_EPOLL is not defined, it means epoll is not available. Return
* NULL */
const grpc_event_engine_vtable *grpc_init_epoll_linux(void) { return NULL; }
+#endif /* defined(GPR_POSIX_SOCKET) */
void grpc_use_signal(int signum) {}
#endif /* !defined(GPR_LINUX_EPOLL) */