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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to