[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

Tue Mar 1 21:52:29 PST 2022

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