[GitHub] [arrow] pitrou commented on a diff in pull request #36372: GH-36369: [C++][FlightRPC] Fix a hang bug in FlightClient::Authenticate*()

["pitrou (via GitHub)" <gi...@apache.org>]

pitrou commented on code in PR #36372:
URL: https://github.com/apache/arrow/pull/36372#discussion_r1246818195


##########
cpp/src/arrow/flight/transport/grpc/grpc_client.cc:
##########
@@ -740,11 +740,18 @@ class GrpcClientImpl : public internal::ClientTransport {
     RETURN_NOT_OK(auth_handler_->Authenticate(&outgoing, &incoming));
     // Explicitly close our side of the connection
     bool finished_writes = stream->WritesDone();
-    RETURN_NOT_OK(FromGrpcStatus(stream->Finish(), &rpc.context));
     if (!finished_writes) {
       return MakeFlightError(FlightStatusCode::Internal,
                              "Could not finish writing before closing");
     }
+    // Drain the read side, as otherwise gRPC Finish() will hang. We
+    // only call Finish() when the client closes the writer or the
+    // reader finishes, so it's OK to assume the client no longer
+    // wants to read and drain the read side.
+    pb::HandshakeResponse response;
+    while (stream->Read(&response)) {
+    }
+    RETURN_NOT_OK(FromGrpcStatus(stream->Finish(), &rpc.context));

Review Comment:
   Can we perhaps factor out all this in a helper method?
   (also, why do we ignore any read response?)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org