zanmato1984 commented on PR #45268: URL: https://github.com/apache/arrow/pull/45268#issuecomment-2650341964
> It does, but I was wondering if the overall design is still correct now that we've made another variable atomic, with updates being done without the mutex. I totally understand the concern here. But I think at this point only the original author can really answer this. I'm making this change based on the following reasons: 1) In the original code there already exist several accesses to `aborted_` without locking. I assume we didn't have a clear definition of how this `aborted_` should be synchronized at the beginning so it has been half locking-based half lockless. (Acero has been fine using it because Acero establishes explicit synchronization from the outside by using future/continuation semantics). 2) AFAICT, The access pattern of `aborted_` is fairly straightforward - there isn't any other state that's relying on it. 3) The test is harsh enough. It triggers the `Abort()` in very fragile timing. Green TSAN gives me a little more confidence. -- 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]
