From 4dff0ee48a91452b5cb23ea8202661b30ff70d11 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 17 Aug 2021 10:50:10 -0700 Subject: [PATCH] Fix bug in CQNext (#27022) Internal bug b/170934515 In some cases, calling CQ::Next will return true without passing up a new tag value, which is very much illegal. My expectation is that this is due to messing up between SHUTDOWN and TIMEOUT in lower layer code that doesn't particularly matter much to most callers, but was being erroneously checked here. --- include/grpcpp/impl/codegen/completion_queue.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/grpcpp/impl/codegen/completion_queue.h b/include/grpcpp/impl/codegen/completion_queue.h index 654ff1c8da9..ba2c8d2be80 100644 --- a/include/grpcpp/impl/codegen/completion_queue.h +++ b/include/grpcpp/impl/codegen/completion_queue.h @@ -175,9 +175,14 @@ class CompletionQueue : private ::grpc::GrpcLibraryCodegen { /// \return true if got an event, false if the queue is fully drained and /// shut down. bool Next(void** tag, bool* ok) { + // Check return type == GOT_EVENT... cases: + // SHUTDOWN - queue has been shutdown, return false. + // TIMEOUT - we passed infinity time => queue has been shutdown, return + // false. + // GOT_EVENT - we actually got an event, return true. return (AsyncNextInternal(tag, ok, ::grpc::g_core_codegen_interface->gpr_inf_future( - GPR_CLOCK_REALTIME)) != SHUTDOWN); + GPR_CLOCK_REALTIME)) == GOT_EVENT); } /// Read from the queue, blocking up to \a deadline (or the queue's shutdown).