[GitHub] [arrow] cyb70289 commented on a diff in pull request #13191: ARROW-16592: [C++][Python][FlightRPC] Finish after failed writes

[GitBox <gi...@apache.org>]

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: github-unsubscribe@arrow.apache.org

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