zanmato1984 commented on PR #44990:
URL: https://github.com/apache/arrow/pull/44990#issuecomment-2757194947

   > I think below might be a incorrect ordering:
   > 
   > Thread1: check `state_->finished.load() == true` in `1` Thread2: 
state->MarkFinishedIfDone, state->final_future.MarkFinished Thread1: 
state_->num_running.fetch_add(1) in `2` Thread1: 
state->final_future.MarkFinished
   
   Hmm, is this assuming two threads executing 
`ReadaheadGenerator::operator()()` simultaneously? I don't think this is not 
possible, as noted in 
https://github.com/apache/arrow/blob/6fee049c816fc772baee9e4f3771e592bcb87c46/cpp/src/arrow/util/async_generator.h#L45-L56
   
   You assumption is what's-so-called "synchronous reentrant", but 
`ReadaheadGenerator` is only "async reentrant": there must be code somewhere 
(I'm not sure where) doing synchronization between 
`ReadaheadGenerator::operator()()` invocations from multiple threads (or the 
caller is just using one thread doing that). You can also see that, suppose 
`ReadaheadGenerator::operator()()` is actually called sync-reentrant-ly, there 
are more serious races in it (see the access to `readahead_queue` for example).


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

Reply via email to