westonpace commented on a change in pull request #9941:
URL: https://github.com/apache/arrow/pull/9941#discussion_r609471342
##########
File path: cpp/src/arrow/util/async_generator.h
##########
@@ -1195,36 +1199,71 @@ class BackgroundGenerator {
ClearQueue();
queue.push(spawn_status);
}
+ task_finished.MarkFinished();
}
}
}
internal::Executor* io_executor;
Iterator<T> it;
- bool running;
+ // True if we are still running in the loop and will be adding more items
to the
+ // queue, don't restart the task if this is true. However, even if this
is false we
+ // might still be running some finish callbacks or marking the finish
future.
+ bool running_in_loop;
+ // If this is false then the background thread is done with everything.
It will not
+ // be running any additional callbacks or marking the finish future.
There is no need
+ // to wait for it when cleaning up.
+ bool running_at_all;
bool finished;
Review comment:
`finished` means that the background thread is done. Any future reads
that would require starting the background generator back up should return an
end token.
`running_at_all` means that not only is it finished but the background
thread is done delivering the last item (we mark finished and "reserve a spot"
in the mutex but can't deliver the item in the mutex) and is not going to be
checking to see if it needs to mark the final future complete.
I will consider an enum and see if the 8 possible states collapse down to
some reasonable subset.
--
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]
