Merge pull request #23223 from grpc/vjpai-patch-1-1

Avoid a use of ReleasableMutexLock

src/cpp/server/server_context.cc

@@ -174,31 +174,32 @@ void ServerContextBase::CompletionOp::FillOps(::grpc::internal::Call* call) {
 }
 
 bool ServerContextBase::CompletionOp::FinalizeResult(void** tag, bool* status) {
-  bool ret = false;
-  grpc_core::ReleasableMutexLock lock(&mu_);
-  if (done_intercepting_) {
-    /* We are done intercepting. */
-    if (has_tag_) {
-      *tag = tag_;
-      ret = true;
-    }
-    Unref();
-    return ret;
-  }
-  finalized_ = true;
+  // Decide whether to call the cancel callback within the lock
+  bool call_cancel;
 
-  // If for some reason the incoming status is false, mark that as a
-  // cancellation.
-  // TODO(vjpai): does this ever happen?
-  if (!*status) {
-    cancelled_ = 1;
-  }
+  {
+    grpc_core::MutexLock lock(&mu_);
+    if (done_intercepting_) {
+      // We are done intercepting.
+      bool has_tag = has_tag_;
+      if (has_tag) {
+        *tag = tag_;
+      }
+      Unref();
+      return has_tag;
+    }
+    finalized_ = true;
 
-  // Decide whether to call the cancel callback before releasing the lock
-  bool call_cancel = (cancelled_ != 0);
+    // If for some reason the incoming status is false, mark that as a
+    // cancellation.
+    // TODO(vjpai): does this ever happen?
+    if (!*status) {
+      cancelled_ = 1;
+    }
 
-  // Release the lock since we may call a callback and interceptors now.
-  lock.Unlock();
+    call_cancel = (cancelled_ != 0);
+    // Release the lock since we may call a callback and interceptors.
+  }
 
   if (call_cancel && callback_controller_ != nullptr) {
     callback_controller_->MaybeCallOnCancel();
@@ -207,15 +208,15 @@ bool ServerContextBase::CompletionOp::FinalizeResult(void** tag, bool* status) {
   interceptor_methods_.AddInterceptionHookPoint(
       ::grpc::experimental::InterceptionHookPoints::POST_RECV_CLOSE);
   if (interceptor_methods_.RunInterceptors()) {
-    /* No interceptors were run */
-    if (has_tag_) {
+    // No interceptors were run
+    bool has_tag = has_tag_;
+    if (has_tag) {
       *tag = tag_;
-      ret = true;
     }
     Unref();
-    return ret;
+    return has_tag;
   }
-  /* There are interceptors to be run. Return false for now */
+  // There are interceptors to be run. Return false for now.
   return false;
 }