From 41622a8e389e8eda38d6d3bfbf34cbf35f437156 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 13 Jun 2016 16:43:14 -0700 Subject: [PATCH] Fix tsan failures --- Makefile | 1 + build.yaml | 1 + src/core/lib/iomgr/ev_epoll_linux.c | 27 ++++++++++++++++++++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4d8b060760a..28d6842c76c 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 85b66d985bb..139ab3e8bca 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 a8a874cd4b0..35a15e00c96 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) */