kou opened a new issue, #36369: URL: https://github.com/apache/arrow/issues/36369
### Describe the bug, including details regarding any error messages, version, and platform. Ballista uses `admin`/`password` for (Basic) authentication: https://arrow.apache.org/ballista/user-guide/flightsql.html#a-name-tool-use-the-driver-in-your-favorite-data-tool The following Apache Arrow C++ Flight RPC client is blocked: ```cpp #include <iostream> #include <arrow/flight/client.h> static arrow::Status run(void) { ARROW_ASSIGN_OR_RAISE(auto location, arrow::flight::Location::Parse("grpc://127.0.0.1:50050")); arrow::flight::FlightClientOptions options; ARROW_ASSIGN_OR_RAISE(auto client, arrow::flight::FlightClient::Connect(location, options)); arrow::flight::FlightCallOptions call_options; ARROW_ASSIGN_OR_RAISE(auto bearer_token, client->AuthenticateBasicToken(call_options, "admin", "password")); return arrow::Status::OK(); } extern "C" int main(void) { auto status = run(); if (!status.ok()) { std::cout << status.ToString() << std::endl; return 1; } return 0; } ``` `stream->Finish()` in `GrpcClientImpl::AuthenticateBasicToken()` is blocked: https://github.com/apache/arrow/blob/main/cpp/src/arrow/flight/transport/grpc/grpc_client.cc#L762 [`grpc::ClientReaderWriter< W, R >::Finish` document](https://grpc.github.io/grpc/cpp/classgrpc_1_1_client_reader_writer.html#a0fe28c9fa3d01f716c9e453245b69b17) refers [`grpc::internal::ClientStreamingInterface::Finish()` document](https://grpc.github.io/grpc/cpp/classgrpc_1_1internal_1_1_client_streaming_interface.html#a567f213cc0a5df2a03becda3e5711e25) and it says: > It is appropriate to call this method exactly once when both: > > * the calling code (client-side) has no more message to send (this can be declared implicitly by calling this method, or explicitly through an earlier call to WritesDone method of the class in use, e.g. [ClientWriterInterface::WritesDone](https://grpc.github.io/grpc/cpp/classgrpc_1_1_client_writer_interface.html#aff19574252338e9ac1b5446e82ed8ac5) or [ClientReaderWriterInterface::WritesDone](https://grpc.github.io/grpc/cpp/classgrpc_1_1_client_reader_writer_interface.html#a52f4e5d5ac7fe0e4995cb337aa0ecfc8)). > * there are no more messages to be received from the server (which can be known implicitly, or explicitly from an earlier call to [ReaderInterface::Read](https://grpc.github.io/grpc/cpp/classgrpc_1_1internal_1_1_reader_interface.html#adfb50ace59b4b9666a46548d3f732ee8) that returned "false"). We call `stream->WritesDone()` explicitly for the former. But we don't call `stream->Read()` until it returns `false`. Should we call `stream->Read()` before we call `stream->Finish()` like the following? ```diff diff --git a/cpp/src/arrow/flight/transport/grpc/grpc_client.cc b/cpp/src/arrow/flight/transport/grpc/grpc_client.cc index 5abecd91a..b382d1e11 100644 --- a/cpp/src/arrow/flight/transport/grpc/grpc_client.cc +++ b/cpp/src/arrow/flight/transport/grpc/grpc_client.cc @@ -740,6 +740,15 @@ 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(); + pb::HandshakeResponse response; + if (!stream->Read(&response)) { + return MakeFlightError(FlightStatusCode::Internal, + "No handshake response from server"); + } + if (stream->Read(&response)) { + return MakeFlightError(FlightStatusCode::Internal, + "Too much handshake response from server"); + } RETURN_NOT_OK(FromGrpcStatus(stream->Finish(), &rpc.context)); if (!finished_writes) { return MakeFlightError(FlightStatusCode::Internal, @@ -759,6 +768,15 @@ class GrpcClientImpl : public internal::ClientTransport { stream = stub_->Handshake(&rpc.context); // Explicitly close our side of the connection. bool finished_writes = stream->WritesDone(); + pb::HandshakeResponse response; + if (!stream->Read(&response)) { + return MakeFlightError(FlightStatusCode::Internal, + "No handshake response from server"); + } + if (stream->Read(&response)) { + return MakeFlightError(FlightStatusCode::Internal, + "Too much handshake response from server"); + } RETURN_NOT_OK(FromGrpcStatus(stream->Finish(), &rpc.context)); if (!finished_writes) { return MakeFlightError(FlightStatusCode::Internal, ``` If we call `stream->Read()` before `stream->Finish()`, the test program isn't blocked. @lidavidm What do you think about this? ### Component(s) C++, FlightRPC -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
