From 0d033b51cedd2653e61ed7d24ebe748d4577d199 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 17 Mar 2017 14:17:55 -0700 Subject: [PATCH 01/10] Test exposing TSAN race Isolated to ev_epoll_linux code Run with (on Linux): ``` tools/run_tests/run_tests.py -l c -c tsan -r ev_epoll -n inf -S --force_default_poller ``` --- test/core/iomgr/ev_epoll_linux_test.c | 85 +++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/test/core/iomgr/ev_epoll_linux_test.c b/test/core/iomgr/ev_epoll_linux_test.c index 4ec959995b2..b16b98028e6 100644 --- a/test/core/iomgr/ev_epoll_linux_test.c +++ b/test/core/iomgr/ev_epoll_linux_test.c @@ -43,6 +43,8 @@ #include #include +#include +#include #include "src/core/lib/iomgr/iomgr.h" #include "src/core/lib/iomgr/workqueue.h" @@ -301,6 +303,88 @@ static void test_add_fd_to_pollset() { grpc_exec_ctx_finish(&exec_ctx); } +typedef struct threading_shared { + gpr_mu *mu; + grpc_pollset *pollset; + grpc_wakeup_fd *wakeup_fd; + grpc_fd *wakeup_desc; + grpc_closure on_wakeup; + int wakeups; +} threading_shared; + +static __thread bool thread_done = false; + +static void test_threading_loop(void *arg) { + threading_shared *shared = arg; + while (!thread_done) { + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_pollset_worker *worker; + gpr_mu_lock(shared->mu); + GPR_ASSERT(GRPC_LOG_IF_ERROR( + "pollset_work", + grpc_pollset_work(&exec_ctx, shared->pollset, &worker, + gpr_now(GPR_CLOCK_MONOTONIC), + gpr_inf_future(GPR_CLOCK_MONOTONIC)))); + gpr_mu_unlock(shared->mu); + grpc_exec_ctx_finish(&exec_ctx); + } +} + +static void test_threading_wakeup(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { + threading_shared *shared = arg; + if (++shared->wakeups > 1000000) { + thread_done = true; + } + GPR_ASSERT(GRPC_LOG_IF_ERROR( + "consume_wakeup", grpc_wakeup_fd_consume_wakeup(shared->wakeup_fd))); + GPR_ASSERT(GRPC_LOG_IF_ERROR("wakeup_next", + grpc_wakeup_fd_wakeup(shared->wakeup_fd))); + grpc_fd_notify_on_read(exec_ctx, shared->wakeup_desc, &shared->on_wakeup); +} + +static void test_threading(void) { + threading_shared shared; + shared.pollset = gpr_zalloc(grpc_pollset_size()); + grpc_pollset_init(shared.pollset, &shared.mu); + + gpr_thd_id thds[100]; + for (size_t i = 0; i < GPR_ARRAY_SIZE(thds); i++) { + gpr_thd_options opt = gpr_thd_options_default(); + gpr_thd_options_set_joinable(&opt); + gpr_thd_new(&thds[i], test_threading_loop, &shared, &opt); + } + grpc_wakeup_fd fd; + GPR_ASSERT(GRPC_LOG_IF_ERROR("wakeup_fd_init", grpc_wakeup_fd_init(&fd))); + shared.wakeup_fd = &fd; + shared.wakeup_desc = grpc_fd_create(fd.read_fd, "wakeup"); + shared.wakeups = 0; + { + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_pollset_add_fd(&exec_ctx, shared.pollset, shared.wakeup_desc); + grpc_fd_notify_on_read( + &exec_ctx, shared.wakeup_desc, + grpc_closure_init(&shared.on_wakeup, test_threading_wakeup, &shared, + grpc_schedule_on_exec_ctx)); + grpc_exec_ctx_finish(&exec_ctx); + } + GPR_ASSERT(GRPC_LOG_IF_ERROR("wakeup_first", + grpc_wakeup_fd_wakeup(shared.wakeup_fd))); + for (size_t i = 0; i < GPR_ARRAY_SIZE(thds); i++) { + gpr_thd_join(thds[i]); + } + fd.read_fd = 0; + grpc_wakeup_fd_destroy(&fd); + { + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_fd_orphan(&exec_ctx, shared.wakeup_desc, NULL, NULL, "done"); + grpc_pollset_shutdown(&exec_ctx, shared.pollset, + grpc_closure_create(destroy_pollset, shared.pollset, + grpc_schedule_on_exec_ctx)); + grpc_exec_ctx_finish(&exec_ctx); + } +} + int main(int argc, char **argv) { const char *poll_strategy = NULL; grpc_test_init(argc, argv); @@ -310,6 +394,7 @@ int main(int argc, char **argv) { if (poll_strategy != NULL && strcmp(poll_strategy, "epoll") == 0) { test_add_fd_to_pollset(); test_pollset_queue_merge_items(); + test_threading(); } else { gpr_log(GPR_INFO, "Skipping the test. The test is only relevant for 'epoll' " From 78058b71aab5a77158275f2ac944dd749295501f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 17 Mar 2017 15:01:56 -0700 Subject: [PATCH 02/10] Make repro more repro-able --- test/core/iomgr/ev_epoll_linux_test.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/core/iomgr/ev_epoll_linux_test.c b/test/core/iomgr/ev_epoll_linux_test.c index b16b98028e6..0717a69cf6c 100644 --- a/test/core/iomgr/ev_epoll_linux_test.c +++ b/test/core/iomgr/ev_epoll_linux_test.c @@ -312,11 +312,11 @@ typedef struct threading_shared { int wakeups; } threading_shared; -static __thread bool thread_done = false; +static __thread int thread_wakeups = 0; static void test_threading_loop(void *arg) { threading_shared *shared = arg; - while (!thread_done) { + while (thread_wakeups < 1000000) { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_pollset_worker *worker; gpr_mu_lock(shared->mu); @@ -333,14 +333,13 @@ static void test_threading_loop(void *arg) { static void test_threading_wakeup(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { threading_shared *shared = arg; - if (++shared->wakeups > 1000000) { - thread_done = true; - } + ++shared->wakeups; + ++thread_wakeups; GPR_ASSERT(GRPC_LOG_IF_ERROR( "consume_wakeup", grpc_wakeup_fd_consume_wakeup(shared->wakeup_fd))); + grpc_fd_notify_on_read(exec_ctx, shared->wakeup_desc, &shared->on_wakeup); GPR_ASSERT(GRPC_LOG_IF_ERROR("wakeup_next", grpc_wakeup_fd_wakeup(shared->wakeup_fd))); - grpc_fd_notify_on_read(exec_ctx, shared->wakeup_desc, &shared->on_wakeup); } static void test_threading(void) { @@ -348,7 +347,7 @@ static void test_threading(void) { shared.pollset = gpr_zalloc(grpc_pollset_size()); grpc_pollset_init(shared.pollset, &shared.mu); - gpr_thd_id thds[100]; + gpr_thd_id thds[2]; for (size_t i = 0; i < GPR_ARRAY_SIZE(thds); i++) { gpr_thd_options opt = gpr_thd_options_default(); gpr_thd_options_set_joinable(&opt); From ca3154d7c8edadcd84e1e199264c30a7f285058e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 17 Mar 2017 15:03:22 -0700 Subject: [PATCH 03/10] Make repro more repro-able --- test/core/iomgr/ev_epoll_linux_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/iomgr/ev_epoll_linux_test.c b/test/core/iomgr/ev_epoll_linux_test.c index 0717a69cf6c..6164c6dc4cd 100644 --- a/test/core/iomgr/ev_epoll_linux_test.c +++ b/test/core/iomgr/ev_epoll_linux_test.c @@ -347,7 +347,7 @@ static void test_threading(void) { shared.pollset = gpr_zalloc(grpc_pollset_size()); grpc_pollset_init(shared.pollset, &shared.mu); - gpr_thd_id thds[2]; + gpr_thd_id thds[10]; for (size_t i = 0; i < GPR_ARRAY_SIZE(thds); i++) { gpr_thd_options opt = gpr_thd_options_default(); gpr_thd_options_set_joinable(&opt); From d2164409343445e1f2de62b210218ad05c75d4c9 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 20 Mar 2017 06:45:39 -0700 Subject: [PATCH 04/10] Switch to no-barrier loads + full barrier cas-s to solve ABA problem --- include/grpc/impl/codegen/atm_gcc_atomic.h | 5 + include/grpc/impl/codegen/atm_gcc_sync.h | 1 + src/core/lib/iomgr/ev_epoll_linux.c | 115 ++++++++------------- 3 files changed, 47 insertions(+), 74 deletions(-) diff --git a/include/grpc/impl/codegen/atm_gcc_atomic.h b/include/grpc/impl/codegen/atm_gcc_atomic.h index 4bd3b257413..a486258c77f 100644 --- a/include/grpc/impl/codegen/atm_gcc_atomic.h +++ b/include/grpc/impl/codegen/atm_gcc_atomic.h @@ -85,6 +85,11 @@ static __inline int gpr_atm_rel_cas(gpr_atm *p, gpr_atm o, gpr_atm n) { p, &o, n, 0, __ATOMIC_RELEASE, __ATOMIC_RELAXED)); } +static __inline int gpr_atm_full_cas(gpr_atm *p, gpr_atm o, gpr_atm n) { + return GPR_ATM_INC_CAS_THEN(__atomic_compare_exchange_n( + p, &o, n, 0, __ATOMIC_ACQ_REL, __ATOMIC_RELAXED)); +} + #define gpr_atm_full_xchg(p, n) \ GPR_ATM_INC_CAS_THEN(__atomic_exchange_n((p), (n), __ATOMIC_ACQ_REL)) diff --git a/include/grpc/impl/codegen/atm_gcc_sync.h b/include/grpc/impl/codegen/atm_gcc_sync.h index 9aa2b43189f..946545a671d 100644 --- a/include/grpc/impl/codegen/atm_gcc_sync.h +++ b/include/grpc/impl/codegen/atm_gcc_sync.h @@ -83,6 +83,7 @@ static __inline void gpr_atm_no_barrier_store(gpr_atm *p, gpr_atm value) { #define gpr_atm_no_barrier_cas(p, o, n) gpr_atm_acq_cas((p), (o), (n)) #define gpr_atm_acq_cas(p, o, n) (__sync_bool_compare_and_swap((p), (o), (n))) #define gpr_atm_rel_cas(p, o, n) gpr_atm_acq_cas((p), (o), (n)) +#define gpr_atm_full_cas(p, o, n) gpr_atm_acq_cas((p), (o), (n)) static __inline gpr_atm gpr_atm_full_xchg(gpr_atm *p, gpr_atm n) { gpr_atm cur; diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c index 11208b9ad13..f1e14e9868f 100644 --- a/src/core/lib/iomgr/ev_epoll_linux.c +++ b/src/core/lib/iomgr/ev_epoll_linux.c @@ -1107,19 +1107,15 @@ static void fd_orphan(grpc_exec_ctx *exec_ctx, grpc_fd *fd, static void notify_on(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, grpc_closure *closure) { while (true) { - /* Fast-path: CLOSURE_NOT_READY -> . - The 'release' cas here matches the 'acquire' load in set_ready and - set_shutdown ensuring that the closure (scheduled by set_ready or - set_shutdown) happens-after the I/O event on the fd */ - if (gpr_atm_rel_cas(state, CLOSURE_NOT_READY, (gpr_atm)closure)) { - return; /* Fast-path successful. Return */ - } - - /* Slowpath. The 'acquire' load matches the 'release' cas in set_ready and - set_shutdown */ - gpr_atm curr = gpr_atm_acq_load(state); + gpr_atm curr = gpr_atm_no_barrier_load(state); switch (curr) { case CLOSURE_NOT_READY: { + /* CLOSURE_NOT_READY -> . */ + if (gpr_atm_no_barrier_cas(state, CLOSURE_NOT_READY, + (gpr_atm)closure)) { + return; /* Successful. Return */ + } + break; /* retry */ } @@ -1165,30 +1161,19 @@ static void notify_on(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, static void set_shutdown(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, grpc_error *shutdown_err) { - /* Try the fast-path first (i.e expect the current value to be - CLOSURE_NOT_READY */ - gpr_atm curr = CLOSURE_NOT_READY; gpr_atm new_state = (gpr_atm)shutdown_err | FD_SHUTDOWN_BIT; while (true) { - /* The 'release' cas here matches the 'acquire' load in notify_on to ensure - that the closure it schedules 'happens-after' the set_shutdown is called - on the fd */ - if (gpr_atm_rel_cas(state, curr, new_state)) { - return; /* Fast-path successful. Return */ - } - /* Fallback to slowpath. This 'acquire' load matches the 'release' cas in notify_on and set_ready */ - curr = gpr_atm_acq_load(state); + gpr_atm curr = gpr_atm_no_barrier_load(state); switch (curr) { - case CLOSURE_READY: { - break; /* retry */ - } - - case CLOSURE_NOT_READY: { + case CLOSURE_READY: + case CLOSURE_NOT_READY: + if (gpr_atm_full_cas(state, curr, new_state)) { + return; /* early out */ + } break; /* retry */ - } default: { /* 'curr' is either a closure or the fd is already shutdown */ @@ -1199,10 +1184,8 @@ static void set_shutdown(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, } /* Fd is not shutdown. Schedule the closure and move the state to - shutdown state. The 'release' cas here matches the 'acquire' load in - notify_on to ensure that the closure it schedules 'happens-after' - the set_shutdown is called on the fd */ - if (gpr_atm_rel_cas(state, curr, new_state)) { + shutdown state. */ + if (gpr_atm_full_cas(state, curr, new_state)) { grpc_closure_sched( exec_ctx, (grpc_closure *)curr, GRPC_ERROR_CREATE_REFERENCING("FD Shutdown", &shutdown_err, 1)); @@ -1220,52 +1203,36 @@ static void set_shutdown(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, } static void set_ready(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state) { - /* Try an optimistic case first (i.e assume current state is - CLOSURE_NOT_READY). - - This 'release' cas matches the 'acquire' load in notify_on ensuring that - any closure (scheduled by notify_on) 'happens-after' the return from - epoll_pwait */ - if (gpr_atm_rel_cas(state, CLOSURE_NOT_READY, CLOSURE_READY)) { - return; /* early out */ - } - - /* The 'acquire' load here matches the 'release' cas in notify_on and - set_shutdown */ - gpr_atm curr = gpr_atm_acq_load(state); - switch (curr) { - case CLOSURE_READY: { - /* Already ready. We are done here */ - break; - } + while (true) { + gpr_atm curr = gpr_atm_acq_load(state); - case CLOSURE_NOT_READY: { - /* The state was not CLOSURE_NOT_READY when we checked initially at the - beginning of this function but now it is CLOSURE_NOT_READY again. - This is only possible if the state transitioned out of - CLOSURE_NOT_READY to either CLOSURE_READY or and then - back to CLOSURE_NOT_READY again (i.e after we entered this function, - the fd became "ready" and the necessary actions were already done). - So there is no need to make the state CLOSURE_READY now */ - break; - } + switch (curr) { + case CLOSURE_READY: { + /* Already ready. We are done here */ + return; + } - default: { - /* 'curr' is either a closure or the fd is shutdown */ - if ((curr & FD_SHUTDOWN_BIT) > 0) { - /* The fd is shutdown. Do nothing */ - } else if (gpr_atm_no_barrier_cas(state, curr, CLOSURE_NOT_READY)) { - /* The cas above was no-barrier since the state is being transitioned to - CLOSURE_NOT_READY; notify_on and set_shutdown do not schedule any - closures when transitioning out of CLOSURE_NO_READY state (i.e there - is no other code that needs to 'happen-after' this) */ + case CLOSURE_NOT_READY: { + if (gpr_atm_no_barrier_cas(state, CLOSURE_NOT_READY, CLOSURE_READY)) { + return; /* early out */ + } + break; /* retry */ + } - grpc_closure_sched(exec_ctx, (grpc_closure *)curr, GRPC_ERROR_NONE); + default: { + /* 'curr' is either a closure or the fd is shutdown */ + if ((curr & FD_SHUTDOWN_BIT) > 0) { + /* The fd is shutdown. Do nothing */ + return; + } else if (gpr_atm_full_barrier_cas(state, curr, CLOSURE_NOT_READY)) { + grpc_closure_sched(exec_ctx, (grpc_closure *)curr, GRPC_ERROR_NONE); + return; + } + /* else the state changed again (only possible by either a racing + set_ready or set_shutdown functions. In both these cases, the closure + would have been scheduled for execution. So we are done here */ + return; } - /* else the state changed again (only possible by either a racing - set_ready or set_shutdown functions. In both these cases, the closure - would have been scheduled for execution. So we are done here */ - break; } } } From cad47dd47a51ca2bd264b47ac182a881f1fd4fac Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 20 Mar 2017 06:59:09 -0700 Subject: [PATCH 05/10] Compile fix --- src/core/lib/iomgr/ev_epoll_linux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c index f1e14e9868f..1e88fe76d3c 100644 --- a/src/core/lib/iomgr/ev_epoll_linux.c +++ b/src/core/lib/iomgr/ev_epoll_linux.c @@ -1224,7 +1224,7 @@ static void set_ready(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state) { if ((curr & FD_SHUTDOWN_BIT) > 0) { /* The fd is shutdown. Do nothing */ return; - } else if (gpr_atm_full_barrier_cas(state, curr, CLOSURE_NOT_READY)) { + } else if (gpr_atm_full_cas(state, curr, CLOSURE_NOT_READY)) { grpc_closure_sched(exec_ctx, (grpc_closure *)curr, GRPC_ERROR_NONE); return; } From 2b4a040d406efb6c27a4ca3732a46145b8ce7d9f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 20 Mar 2017 09:21:20 -0700 Subject: [PATCH 06/10] Fix race? --- src/core/lib/iomgr/ev_epoll_linux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c index 1e88fe76d3c..3b35491df9f 100644 --- a/src/core/lib/iomgr/ev_epoll_linux.c +++ b/src/core/lib/iomgr/ev_epoll_linux.c @@ -1111,7 +1111,7 @@ static void notify_on(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, switch (curr) { case CLOSURE_NOT_READY: { /* CLOSURE_NOT_READY -> . */ - if (gpr_atm_no_barrier_cas(state, CLOSURE_NOT_READY, + if (gpr_atm_full_cas(state, CLOSURE_NOT_READY, (gpr_atm)closure)) { return; /* Successful. Return */ } From b2d8e03c5e1928464efc8251f9406cf8506f8973 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 21 Mar 2017 11:01:48 -0700 Subject: [PATCH 07/10] Weaken some barriers that were stronger than necessary --- src/core/lib/iomgr/ev_epoll_linux.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c index 3b35491df9f..d5acdf0d4b4 100644 --- a/src/core/lib/iomgr/ev_epoll_linux.c +++ b/src/core/lib/iomgr/ev_epoll_linux.c @@ -1111,8 +1111,7 @@ static void notify_on(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, switch (curr) { case CLOSURE_NOT_READY: { /* CLOSURE_NOT_READY -> . */ - if (gpr_atm_full_cas(state, CLOSURE_NOT_READY, - (gpr_atm)closure)) { + if (gpr_atm_rel_cas(state, CLOSURE_NOT_READY, (gpr_atm)closure)) { return; /* Successful. Return */ } @@ -1204,7 +1203,7 @@ static void set_shutdown(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, static void set_ready(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state) { while (true) { - gpr_atm curr = gpr_atm_acq_load(state); + gpr_atm curr = gpr_atm_no_barrier_load(state); switch (curr) { case CLOSURE_READY: { From 39311f5599f1206e67c1b616e81b8e4d2afed1a2 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 29 Mar 2017 09:25:06 -0700 Subject: [PATCH 08/10] Fix comments --- src/core/lib/iomgr/ev_epoll_linux.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c index c4c67df381c..97f8f82fed9 100644 --- a/src/core/lib/iomgr/ev_epoll_linux.c +++ b/src/core/lib/iomgr/ev_epoll_linux.c @@ -1135,7 +1135,7 @@ static void notify_on(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, is no other code that needs to 'happen-after' this) */ if (gpr_atm_no_barrier_cas(state, CLOSURE_READY, CLOSURE_NOT_READY)) { grpc_closure_sched(exec_ctx, closure, GRPC_ERROR_NONE); - return; /* Slow-path successful. Return */ + return; /* Successful. Return */ } break; /* retry */ @@ -1169,8 +1169,6 @@ static void set_shutdown(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, gpr_atm new_state = (gpr_atm)shutdown_err | FD_SHUTDOWN_BIT; while (true) { - /* Fallback to slowpath. This 'acquire' load matches the 'release' cas in - notify_on and set_ready */ gpr_atm curr = gpr_atm_no_barrier_load(state); switch (curr) { case CLOSURE_READY: From d51bbeb1ebc3afe6ce6f614eda3137fd39a70592 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 29 Mar 2017 13:46:12 -0700 Subject: [PATCH 09/10] Fix leak --- test/core/iomgr/ev_epoll_linux_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/test/core/iomgr/ev_epoll_linux_test.c b/test/core/iomgr/ev_epoll_linux_test.c index 1a600e34d66..5f8124aedae 100644 --- a/test/core/iomgr/ev_epoll_linux_test.c +++ b/test/core/iomgr/ev_epoll_linux_test.c @@ -393,6 +393,7 @@ static void test_threading(void) { grpc_schedule_on_exec_ctx)); grpc_exec_ctx_finish(&exec_ctx); } + gpr_free(shared.pollset); } int main(int argc, char **argv) { From c212e6cc5e005526a80320ac95424e46dd341ad5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 30 Mar 2017 08:26:21 -0700 Subject: [PATCH 10/10] Fix barrier, comment --- src/core/lib/iomgr/ev_epoll_linux.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c index 97f8f82fed9..f6372c0f3f6 100644 --- a/src/core/lib/iomgr/ev_epoll_linux.c +++ b/src/core/lib/iomgr/ev_epoll_linux.c @@ -1173,9 +1173,9 @@ static void set_shutdown(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state, switch (curr) { case CLOSURE_READY: case CLOSURE_NOT_READY: - /* Release cas to pair with a set_ready performing a load of the - shutdown state later */ - if (gpr_atm_rel_cas(state, curr, new_state)) { + /* Need a full barrier here so that the initial load in notify_on + doesn't need a barrier */ + if (gpr_atm_full_cas(state, curr, new_state)) { return; /* early out */ } break; /* retry */