lidavidm commented on a change in pull request #9941:
URL: https://github.com/apache/arrow/pull/9941#discussion_r609110316



##########
File path: cpp/src/arrow/util/async_generator.h
##########
@@ -1195,25 +1199,48 @@ class BackgroundGenerator {
             ClearQueue();
             queue.push(spawn_status);
           }
+          task_finished.MarkFinished();
         }
       }
     }
 
     internal::Executor* io_executor;
     Iterator<T> it;
+    bool started;
     bool running;
     bool finished;
+    bool should_shutdown;
     int max_q;
     int q_restart;
     std::queue<Result<T>> queue;
     util::optional<Future<T>> waiting_future;
     util::Mutex mutex;
+    Future<> task_finished;
+  };
+
+  struct Cleanup {
+    explicit Cleanup(State* state) : state(state) {}
+    ~Cleanup() {
+      Future<> finish_fut;
+      {
+        auto lock = state->mutex.Lock();
+        if (!state->started) {

Review comment:
       Hmm, you're right, I got myself mixed up before. I guess the concern is 
actually - what if (a) the background task is currently running and (b) it's 
not the first time it's run - it looks like task_finished will have been 
completed the first time we ran the background task, so this won't actually 
wait for the background task to end. Maybe task_finished needs to be 
reinitialized when the task starts?




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