westonpace commented on a change in pull request #7179:
URL: https://github.com/apache/arrow/pull/7179#discussion_r543011950



##########
File path: cpp/src/arrow/util/future.h
##########
@@ -238,6 +242,22 @@ class Future {
     return impl_->Wait(seconds);
   }
 
+  template <typename... CancelArgs>
+  bool Cancel(CancelArgs&&... args) {
+    auto& stop_token = impl_->stop_token_;

Review comment:
       What if the future has already been marked completed?  This could happen 
with a timing issue (user requests cancel at the same time the operation is 
finishing) or when we mix this in with continuations it could happen where a 
chain of futures is cancelled from the end, the cancellation will propagate 
backwards through the chain, and the first few futures may have already 
completed.
   
   This second case we could address when merging that change in too.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to