clayborg added a comment.

HostThread::Reset() is a bad function and should not be directly used by anyone 
other that Join() or Detach(). Just clearing the thread ID and setting the 
result to zero is bad and means we will leak a thread if no one else joins it. 
If someone else already called Detach() on the thread, then Reset() is ok. Also 
calling m_private_state_thread.Cancel() is equally bad.

The real question is why is the cancel timing out?

I agree that the call to "m_private_state_thread.Reset();" should be removed 
from Process::RunPrivateStateThread(). But we should look at why we need to 
call Cancel in the first place. We should never need to cancel our own thread 
unless something really bad has gone on. So it seems like the real bug here is 
bad thread synchronization leading to issues.

I don't like the fact that we are not copying the thread anymore. We have code 
that sometimes needs to spin up an extra private state thread and I wouldn't 
want the code that changes the m_private_state_thread after backing up an older 
one, cause this function to call IsJoinable(), Cancel() and Join() on different 
objects. So it seems like the only needed fix here is to not call 
"m_private_state_thread.Reset();" in Process::RunPrivateStateThread(), all 
other changes can be removed?


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