JDevlieghere added a comment. In D120762#3352628 <https://reviews.llvm.org/D120762#3352628>, @labath wrote:
> These kinds of changes rarely fix a bug -- usually they just change a > (detectable) data race into some nondeterministic runtime behavior. > > That's because, even though e.g. `IsActive` can compute its result in a > race-free manner, there's nothing preventing someone from invalidating it one > nanosecond after the function returns (and before anyone manages to act on > it). And if it isn't possible for someone to concurrently change the values > that IsActive depends on, then we wouldn't have a data race in the first > place. > > Most of the time, that means the result of the function is useless. I haven't > checked tried (yet) to figure out how could that problem manifest itself in > this case (the iohandler logic is fairly complex), but I'm pretty sure that > this should be fixed at a higher level. I think in this particular case the race is fairly "harmless". In the read/write scenario, at worst we execute the run-loop in `IOHandlerProcessSTDIO` once more than we should. In case of a write/write race we are concurrently writing the same value. Anyway, that's not an excuse not to fix the underlying issue. I think the updated patch should take care of that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120762/new/ https://reviews.llvm.org/D120762 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
