[Lldb-commits] [PATCH] D19122: LLDB: Fixed race condition on timeout when stopping private state thread

[Cameron via lldb-commits]

cameron314 added a comment.

Hmm, interesting. Yes, it should not be timing out in the first place. That's likely a bug on our side, still to be determined. I'm looking into it. Regardless, it led us to find and fix this race :-)

I think it still makes sense for LLDB to do a `Cancel()` as a last-ditch effort on timing out. But `Cancel()` is fundamentally very dangerous, so then again, maybe not.

Can the code that spins up the extra thread (RunThreadPlan) run on a different thread than ControlPrivateStateThread runs on? I was under the (perhaps entirely false!) impression that the Process object was used by only one thread at a time (with the exception of the private state thread(s), of course, but that thread shouldn't touch its own control variables).

If that's not the case, then looking at the RunThreadPlan code, there's lots more races on m_private_state_thread alone (e.g. calling EqualsThread on it while it could be assigned from another thread). And even just creating a copy is not thread safe, for example, since it could be reassigned from another thread at the same time... you'd need `m_private_state_thread` to be wrap its shared pointer with `std::atomic`.


Repository:
  rL LLVM

http://reviews.llvm.org/D19122