Merge github.com:grpc/grpc into bugscrub1-proposedfix1

1 files changed, 28 insertions, 11 deletions

diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c
index d5acdf0d4b..c4c67df381 100644
--- a/src/core/lib/iomgr/ev_epoll_linux.c
+++ b/src/core/lib/iomgr/ev_epoll_linux.c
@@ -321,7 +321,7 @@ static bool append_error(grpc_error **composite, grpc_error *error,
                          const char *desc) {
   if (error == GRPC_ERROR_NONE) return true;
   if (*composite == GRPC_ERROR_NONE) {
-    *composite = GRPC_ERROR_CREATE(desc);
+    *composite = GRPC_ERROR_CREATE_FROM_COPIED_STRING(desc);
   }
   *composite = grpc_error_add_child(*composite, error);
   return false;
@@ -1110,7 +1110,13 @@ static void notify_on(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state,
     gpr_atm curr = gpr_atm_no_barrier_load(state);
     switch (curr) {
       case CLOSURE_NOT_READY: {
-        /* CLOSURE_NOT_READY -> <closure>. */
+        /* CLOSURE_NOT_READY -> <closure>.
+
+           We're guaranteed by API that there's an acquire barrier before here,
+           so there's no need to double-dip and this can be a release-only.
+
+           The release itself pairs with the acquire half of a set_ready full
+           barrier. */
         if (gpr_atm_rel_cas(state, CLOSURE_NOT_READY, (gpr_atm)closure)) {
           return; /* Successful. Return */
         }
@@ -1141,9 +1147,9 @@ static void notify_on(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state,
            schedule the closure with the shutdown error */
         if ((curr & FD_SHUTDOWN_BIT) > 0) {
           grpc_error *shutdown_err = (grpc_error *)(curr & ~FD_SHUTDOWN_BIT);
-          grpc_closure_sched(
-              exec_ctx, closure,
-              GRPC_ERROR_CREATE_REFERENCING("FD Shutdown", &shutdown_err, 1));
+          grpc_closure_sched(exec_ctx, closure,
+                             GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING(
+                                 "FD Shutdown", &shutdown_err, 1));
           return;
         }
 
@@ -1169,7 +1175,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:
-        if (gpr_atm_full_cas(state, curr, new_state)) {
+        /* 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)) {
           return; /* early out */
         }
         break; /* retry */
@@ -1183,11 +1191,14 @@ 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. */
+           shutdown state.
+           Needs an acquire to pair with setting the closure (and get a
+           happens-after on that edge), and a release to pair with anything
+           loading the 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));
+          grpc_closure_sched(exec_ctx, (grpc_closure *)curr,
+                             GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING(
+                                 "FD Shutdown", &shutdown_err, 1));
           return;
         }
 
@@ -1212,6 +1223,8 @@ static void set_ready(grpc_exec_ctx *exec_ctx, grpc_fd *fd, gpr_atm *state) {
       }
 
       case CLOSURE_NOT_READY: {
+        /* No barrier required as we're transitioning to a state that does not
+           involve a closure */
         if (gpr_atm_no_barrier_cas(state, CLOSURE_NOT_READY, CLOSURE_READY)) {
           return; /* early out */
         }
@@ -1223,7 +1236,11 @@ 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_cas(state, curr, CLOSURE_NOT_READY)) {
+        }
+        /* Full cas: acquire pairs with this cas' release in the event of a
+           spurious set_ready; release pairs with this or the acquire in
+           notify_on (or set_shutdown) */
+        else if (gpr_atm_full_cas(state, curr, CLOSURE_NOT_READY)) {
           grpc_closure_sched(exec_ctx, (grpc_closure *)curr, GRPC_ERROR_NONE);
           return;
         }