Merge pull request #12372 from vjpai/armalarmadingdong

Core API change: Decouple alarm construction from setting to avoid races in MT code

1 files changed, 26 insertions, 10 deletions

diff --git a/include/grpc++/alarm.h b/include/grpc++/alarm.h
index ed8dacbc94..2d88d868e5 100644
--- a/include/grpc++/alarm.h
+++ b/include/grpc++/alarm.h
@@ -37,20 +37,33 @@ class CompletionQueue;
 /// A thin wrapper around \a grpc_alarm (see / \a / src/core/surface/alarm.h).
 class Alarm : private GrpcLibraryCodegen {
  public:
-  /// Create a completion queue alarm instance associated to \a cq.
-  ///
-  /// Once the alarm expires (at \a deadline) or it's cancelled (see \a Cancel),
-  /// an event with tag \a tag will be added to \a cq. If the alarm expired, the
-  /// event's success bit will be true, false otherwise (ie, upon cancellation).
+  /// Create an unset completion queue alarm
+  Alarm() : tag_(nullptr), alarm_(grpc_alarm_create(nullptr)) {}
+
+  /// DEPRECATED: Create and set a completion queue alarm instance associated to
+  /// \a cq.
+  /// This form is deprecated because it is inherently racy.
   /// \internal We rely on the presence of \a cq for grpc initialization. If \a
   /// cq were ever to be removed, a reference to a static
   /// internal::GrpcLibraryInitializer instance would need to be introduced
   /// here. \endinternal.
   template <typename T>
   Alarm(CompletionQueue* cq, const T& deadline, void* tag)
-      : tag_(tag),
-        alarm_(grpc_alarm_create(cq->cq(), TimePoint<T>(deadline).raw_time(),
-                                 static_cast<void*>(&tag_))) {}
+      : tag_(tag), alarm_(grpc_alarm_create(nullptr)) {
+    grpc_alarm_set(alarm_, cq->cq(), TimePoint<T>(deadline).raw_time(),
+                   static_cast<void*>(&tag_), nullptr);
+  }
+
+  /// Trigger an alarm instance on completion queue \a cq at the specified time.
+  /// Once the alarm expires (at \a deadline) or it's cancelled (see \a Cancel),
+  /// an event with tag \a tag will be added to \a cq. If the alarm expired, the
+  /// event's success bit will be true, false otherwise (ie, upon cancellation).
+  template <typename T>
+  void Set(CompletionQueue* cq, const T& deadline, void* tag) {
+    tag_.Set(tag);
+    grpc_alarm_set(alarm_, cq->cq(), TimePoint<T>(deadline).raw_time(),
+                   static_cast<void*>(&tag_), nullptr);
+  }
 
   /// Alarms aren't copyable.
   Alarm(const Alarm&) = delete;
@@ -69,17 +82,20 @@ class Alarm : private GrpcLibraryCodegen {
 
   /// Destroy the given completion queue alarm, cancelling it in the process.
   ~Alarm() {
-    if (alarm_ != nullptr) grpc_alarm_destroy(alarm_);
+    if (alarm_ != nullptr) grpc_alarm_destroy(alarm_, nullptr);
   }
 
   /// Cancel a completion queue alarm. Calling this function over an alarm that
   /// has already fired has no effect.
-  void Cancel() { grpc_alarm_cancel(alarm_); }
+  void Cancel() {
+    if (alarm_ != nullptr) grpc_alarm_cancel(alarm_, nullptr);
+  }
 
  private:
   class AlarmEntry : public CompletionQueueTag {
    public:
     AlarmEntry(void* tag) : tag_(tag) {}
+    void Set(void* tag) { tag_ = tag; }
     bool FinalizeResult(void** tag, bool* status) override {
       *tag = tag_;
       return true;