From e1e7042919f698c2c3ea9a507c84b0ec859ed547 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Sat, 13 Jan 2018 23:53:01 -0800 Subject: [PATCH] Restructure to simplify, harden, and avoid forward declaration --- include/grpc++/alarm.h | 6 +----- src/cpp/common/alarm.cc | 42 ++++++++++++++++++----------------------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/include/grpc++/alarm.h b/include/grpc++/alarm.h index 4660b04cfa4..37d41892011 100644 --- a/include/grpc++/alarm.h +++ b/include/grpc++/alarm.h @@ -30,10 +30,6 @@ namespace grpc { -namespace internal { -class AlarmImpl; -} - /// A thin wrapper around \a grpc_alarm (see / \a / src/core/surface/alarm.h). class Alarm : private GrpcLibraryCodegen { public: @@ -83,7 +79,7 @@ class Alarm : private GrpcLibraryCodegen { private: void SetInternal(CompletionQueue* cq, gpr_timespec deadline, void* tag); - internal::AlarmImpl* alarm_; + internal::CompletionQueueTag* alarm_; }; } // namespace grpc diff --git a/src/cpp/common/alarm.cc b/src/cpp/common/alarm.cc index a4c54ed8a31..0c912900b53 100644 --- a/src/cpp/common/alarm.cc +++ b/src/cpp/common/alarm.cc @@ -34,7 +34,7 @@ namespace grpc { namespace internal { -class AlarmImpl { +class AlarmImpl : public CompletionQueueTag { public: AlarmImpl() : cq_(nullptr), tag_(nullptr) { gpr_ref_init(&refs_, 1); @@ -45,10 +45,8 @@ class AlarmImpl { AlarmImpl* alarm = static_cast(arg); alarm->Ref(); grpc_cq_end_op( - alarm->cq_, &alarm->tag_, error, + alarm->cq_, alarm, error, [](void* arg, grpc_cq_completion* completion) { - AlarmImpl* alarm = static_cast(arg); - alarm->Unref(); }, arg, &alarm->completion_); }, @@ -60,12 +58,17 @@ class AlarmImpl { GRPC_CQ_INTERNAL_UNREF(cq_, "alarm"); } } + bool FinalizeResult(void** tag, bool* status) override { + *tag = tag_; + Unref(); + return true; + } void Set(CompletionQueue* cq, gpr_timespec deadline, void* tag) { grpc_core::ExecCtx exec_ctx; GRPC_CQ_INTERNAL_REF(cq->cq(), "alarm"); cq_ = cq->cq(); - tag_.Set(tag); - GPR_ASSERT(grpc_cq_begin_op(cq_, &tag_)); + tag_ = tag; + GPR_ASSERT(grpc_cq_begin_op(cq_, this)); grpc_timer_init(&timer_, grpc_timespec_to_millis_round_up(deadline), &on_alarm_); } @@ -77,21 +80,7 @@ class AlarmImpl { Cancel(); Unref(); } - private: - class AlarmEntry : public internal::CompletionQueueTag { - public: - AlarmEntry(void* tag) : tag_(tag) {} - void Set(void* tag) { tag_ = tag; } - bool FinalizeResult(void** tag, bool* status) override { - *tag = tag_; - return true; - } - - private: - void* tag_; - }; - void Ref() { gpr_ref(&refs_); } void Unref() { if (gpr_unref(&refs_)) { @@ -105,7 +94,7 @@ class AlarmImpl { grpc_cq_completion completion_; // completion queue where events about this alarm will be posted grpc_completion_queue* cq_; - AlarmEntry tag_; + void* tag_; }; } // namespace internal @@ -116,14 +105,19 @@ Alarm::Alarm() : alarm_(new internal::AlarmImpl()) { } void Alarm::SetInternal(CompletionQueue* cq, gpr_timespec deadline, void* tag) { - alarm_->Set(cq, deadline, tag); + // Note that we know that alarm_ is actually an internal::AlarmImpl + // but we declared it as the base pointer to avoid a forward declaration + // or exposing core data structures in the C++ public headers. + // Thus it is safe to use a static_cast to the subclass here, and the + // C++ style guide allows us to do so in this case + static_cast(alarm_)->Set(cq, deadline, tag); } Alarm::~Alarm() { if (alarm_ != nullptr) { - alarm_->Destroy(); + static_cast(alarm_)->Destroy(); } } -void Alarm::Cancel() { alarm_->Cancel(); } +void Alarm::Cancel() { static_cast(alarm_)->Cancel(); } } // namespace grpc