clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

This looks like it would work for normal operation. I am not sure it will work 
when an extra private state thread is spun up. In certain circumstances we need 
to create more private state threads. This happens when you have an expression 
that is run from the private state thread. Since the private state thread is 
what controls the actual process, we can't run an expression from the current 
private state thread, so we spin up new ones. The current code is doing some 
tricky things to deal with this, and that was part of the reason 
Process::ControlPrivateStateThread() was making a copy of the current value of 
"m_private_state_thread" into a local variable named "private_state_thread":

  HostThread private_state_thread(m_private_state_thread);

See the code in "Process::RunThreadPlan()" around the code:

  if (m_private_state_thread.EqualsThread(Host::GetCurrentThread()))

The new loop as written in Process::ControlPrivateStateThread() could end up 
using m_private_state_thread with differing contents in the "if 
(m_private_state_thread.IsJoinable())" if statement. Jim Ingham made these 
changes, so we should add him to the reviewer list. I am going to mark as 
"Request Changes" so we can address any such issues, but Jim should chime in on 
this before we proceed.

The way this normally happens is an expression is being run and while handling 
the expression on the private state thread, we need to run another expression 
(like a call to "mmap" in the debugged process so we can allocate memory), so 
we need to spin up another private state thread to handle the needed starts and 
stops. Only one of these threads will be actively grabbing events at a single 
time, so your patch might just work, but I want to get some more eyes on this 
to be sure.

Please add Jim Ingham as a reviewer.


http://reviews.llvm.org/D21296



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

Reply via email to