Merge pull request #2470 from ctiller/i-want-to-wait-free

Lock free requested call fulfillment path

1 files changed, 32 insertions, 7 deletions

diff --git a/src/core/iomgr/pollset_posix.c b/src/core/iomgr/pollset_posix.c
index 7afff58eee..efb301d81c 100644
--- a/src/core/iomgr/pollset_posix.c
+++ b/src/core/iomgr/pollset_posix.c
@@ -105,14 +105,28 @@ void grpc_pollset_init(grpc_pollset *pollset) {
 
 void grpc_pollset_add_fd(grpc_pollset *pollset, grpc_fd *fd) {
   gpr_mu_lock(&pollset->mu);
-  pollset->vtable->add_fd(pollset, fd);
+  pollset->vtable->add_fd(pollset, fd, 1);
+  /* the following (enabled only in debug) will reacquire and then release
+     our lock - meaning that if the unlocking flag passed to del_fd above is
+     not respected, the code will deadlock (in a way that we have a chance of
+     debugging) */
+#ifndef NDEBUG
+  gpr_mu_lock(&pollset->mu);
   gpr_mu_unlock(&pollset->mu);
+#endif
 }
 
 void grpc_pollset_del_fd(grpc_pollset *pollset, grpc_fd *fd) {
   gpr_mu_lock(&pollset->mu);
-  pollset->vtable->del_fd(pollset, fd);
+  pollset->vtable->del_fd(pollset, fd, 1);
+  /* the following (enabled only in debug) will reacquire and then release
+     our lock - meaning that if the unlocking flag passed to del_fd above is
+     not respected, the code will deadlock (in a way that we have a chance of
+     debugging) */
+#ifndef NDEBUG
+  gpr_mu_lock(&pollset->mu);
   gpr_mu_unlock(&pollset->mu);
+#endif
 }
 
 static void finish_shutdown(grpc_pollset *pollset) {
@@ -257,7 +271,7 @@ static void basic_do_promote(void *args, int success) {
   } else if (grpc_fd_is_orphaned(fd)) {
     /* Don't try to add it to anything, we'll drop our ref on it below */
   } else if (pollset->vtable != original_vtable) {
-    pollset->vtable->add_fd(pollset, fd);
+    pollset->vtable->add_fd(pollset, fd, 0);
   } else if (fd != pollset->data.ptr) {
     grpc_fd *fds[2];
     fds[0] = pollset->data.ptr;
@@ -287,10 +301,11 @@ static void basic_do_promote(void *args, int success) {
   GRPC_FD_UNREF(fd, "basicpoll_add");
 }
 
-static void basic_pollset_add_fd(grpc_pollset *pollset, grpc_fd *fd) {
+static void basic_pollset_add_fd(grpc_pollset *pollset, grpc_fd *fd,
+                                 int and_unlock_pollset) {
   grpc_unary_promote_args *up_args;
   GPR_ASSERT(fd);
-  if (fd == pollset->data.ptr) return;
+  if (fd == pollset->data.ptr) goto exit;
 
   if (!pollset->counter) {
     /* Fast path -- no in flight cbs */
@@ -313,7 +328,7 @@ static void basic_pollset_add_fd(grpc_pollset *pollset, grpc_fd *fd) {
       pollset->data.ptr = fd;
       GRPC_FD_REF(fd, "basicpoll");
     }
-    return;
+    goto exit;
   }
 
   /* Now we need to promote. This needs to happen when we're not polling. Since
@@ -329,14 +344,24 @@ static void basic_pollset_add_fd(grpc_pollset *pollset, grpc_fd *fd) {
   grpc_iomgr_add_callback(&up_args->promotion_closure);
 
   grpc_pollset_kick(pollset);
+
+exit:
+  if (and_unlock_pollset) {
+    gpr_mu_unlock(&pollset->mu);
+  }
 }
 
-static void basic_pollset_del_fd(grpc_pollset *pollset, grpc_fd *fd) {
+static void basic_pollset_del_fd(grpc_pollset *pollset, grpc_fd *fd,
+                                 int and_unlock_pollset) {
   GPR_ASSERT(fd);
   if (fd == pollset->data.ptr) {
     GRPC_FD_UNREF(pollset->data.ptr, "basicpoll");
     pollset->data.ptr = NULL;
   }
+
+  if (and_unlock_pollset) {
+    gpr_mu_unlock(&pollset->mu);
+  }
 }
 
 static void basic_pollset_maybe_work(grpc_pollset *pollset,