From 3e0a46a1c4297b2d8fd0f05162bf551f3bae78b5 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 12 Mar 2015 05:16:31 -0700 Subject: [PATCH] Change behavior to properly account for possibility of NULL tag. This can happen if the tag is actually an integer being typecast to void* To avoid breaking the API of existing Next calls, I've made a new AsyncNext method with a tri-state return that indicates whether there is a shutdown, an actual event, or a timeout. Still needs proper testing for the AsyncNext method specifically. --- include/grpc++/completion_queue.h | 15 ++++++++++++--- src/cpp/common/completion_queue.cc | 11 +++++------ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/include/grpc++/completion_queue.h b/include/grpc++/completion_queue.h index db6e6d41f31..ffa53698bbc 100644 --- a/include/grpc++/completion_queue.h +++ b/include/grpc++/completion_queue.h @@ -76,10 +76,19 @@ class CompletionQueue { explicit CompletionQueue(grpc_completion_queue *take); ~CompletionQueue(); + // Tri-state return for Next: SHUTDOWN, GOT_EVENT, TIMEOUT + enum NextStatus {SHUTDOWN, GOT_EVENT, TIMEOUT}; + // Blocking (until deadline) read from queue. - // Returns false if the queue is ready for destruction, true otherwise - // If the deadline passed, *tag will be null - bool Next(void **tag, bool *ok, gpr_timespec deadline=gpr_inf_future); + // Returns false if the queue is ready for destruction, true if event + + bool Next(void **tag, bool *ok) { + return (AsyncNext(tag,ok,gpr_inf_future) != SHUTDOWN); + } + + // Nonblocking (until deadline) read from queue. + // Cannot rely on result of tag or ok if return is TIMEOUT + NextStatus AsyncNext(void **tag, bool *ok, gpr_timespec deadline); // Shutdown has to be called, and the CompletionQueue can only be // destructed when false is returned from Next(). diff --git a/src/cpp/common/completion_queue.cc b/src/cpp/common/completion_queue.cc index c37e97909b8..2913298afed 100644 --- a/src/cpp/common/completion_queue.cc +++ b/src/cpp/common/completion_queue.cc @@ -57,24 +57,23 @@ class EventDeleter { } }; -bool CompletionQueue::Next(void** tag, bool* ok, gpr_timespec deadline) { +CompletionQueue::NextStatus CompletionQueue::AsyncNext(void** tag, bool* ok, + gpr_timespec deadline) { std::unique_ptr ev; for (;;) { ev.reset(grpc_completion_queue_next(cq_, deadline)); if (!ev) { /* got a NULL back because deadline passed */ - *ok = true; - *tag = nullptr; - return true; + return TIMEOUT; } if (ev->type == GRPC_QUEUE_SHUTDOWN) { - return false; + return SHUTDOWN; } auto cq_tag = static_cast(ev->tag); *ok = ev->data.op_complete == GRPC_OP_OK; *tag = cq_tag; if (cq_tag->FinalizeResult(tag, ok)) { - return true; + return GOT_EVENT; } } }