cyb70289 commented on code in PR #13191:
URL: https://github.com/apache/arrow/pull/13191#discussion_r877704387
##########
cpp/src/arrow/flight/transport/grpc/grpc_client.cc:
##########
@@ -350,16 +347,7 @@ class WritableDataStream : public
FinishableDataStream<Stream, ReadPayload> {
Status DoFinish() override {
// This may be used concurrently by reader/writer side of a
// stream, so it needs to be protected.
- std::lock_guard<std::mutex> guard(finish_mutex_);
-
- // Now that we're shared between a reader and writer, we need to
- // protect ourselves from being called while there's an
- // outstanding read.
- std::unique_lock<std::mutex> read_guard(read_mutex_, std::try_to_lock);
- if (!read_guard.owns_lock()) {
- return MakeFlightError(FlightStatusCode::Internal,
- "Cannot close stream with pending read
operation.");
- }
+ std::lock_guard<std::mutex> guard(read_mutex_);
Review Comment:
Compared with original code, looks we will wait for reading finishes instead
of fail immediately. Is it a problem?
Nit: IIUC `read_mutex` is to guard concurrent `read` to `finish`. If that's
the case, the name may be a bit confusing, it sounds like to guard concurrent
`read` to `read`. Is `finish_mutex` a better name?
--
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]
