cameron314 created this revision.
cameron314 added reviewers: clayborg, labath, zturner.
cameron314 added a subscriber: lldb-commits.
This is the follow-up to D19122, which was accepted but subsequently reverted
due to a bug it introduced (that I didn't see during local testing on Windows
but which manifested quite often on Linux). That bug (a race between the
Process object was being destroyed and the thread terminating, caused by the
join not being done under certain conditions) is fixed in this version of the
patch.
This patch fixes various races between the time the private state thread is
signaled to exit and the time it actually exits (during which it no longer
responds to events). Previously, this was consistently causing 2-second timeout
delays on process detach/stop for us.
This also prevents crashes that were caused by the thread controlling its own
owning pointer while the controller was using it (copying the thread wrapper is
not enough to mitigate this, since the internal thread object was getting reset
anyway). Again, we were seeing this consistently.
For what it's worth, I've run the test suite with this change (on Linux)
without any regressions, and the number of reruns dropped from 15 to 0 for me
(though that last part may be coincidence).
http://reviews.llvm.org/D21296
Files:
include/lldb/Target/Process.h
source/Target/Process.cpp
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -4088,7 +4088,7 @@
void
Process::StopPrivateStateThread ()
{
- if (PrivateStateThreadIsValid ())
+ if (m_private_state_thread.IsJoinable ())
ControlPrivateStateThread (eBroadcastInternalStateControlStop);
else
{
@@ -4110,21 +4110,23 @@
if (log)
log->Printf ("Process::%s (signal = %d)", __FUNCTION__, signal);
- // Signal the private state thread. First we should copy this is case the
- // thread starts exiting since the private state thread will NULL this out
- // when it exits
+ // Signal the private state thread
+ if (m_private_state_thread.IsJoinable())
{
- HostThread private_state_thread(m_private_state_thread);
- if (private_state_thread.IsJoinable())
- {
- if (log)
- log->Printf ("Sending control event of type: %d.", signal);
- // Send the control event and wait for the receipt or for the private state
- // thread to exit
- std::shared_ptr<EventDataReceipt> event_receipt_sp(new EventDataReceipt());
- m_private_state_control_broadcaster.BroadcastEvent(signal, event_receipt_sp);
+ // Broadcast the event.
+ // It is important to do this outside of the if below, because
+ // it's possible that the thread state is invalid but that the
+ // thread is waiting on a control event instead of simply being
+ // on its way out (this should not happen, but it apparently can).
+ if (log)
+ log->Printf ("Sending control event of type: %d.", signal);
+ std::shared_ptr<EventDataReceipt> event_receipt_sp(new EventDataReceipt());
+ m_private_state_control_broadcaster.BroadcastEvent(signal, event_receipt_sp);
- bool receipt_received = false;
+ // Wait for the event receipt or for the private state thread to exit
+ bool receipt_received = false;
+ if (PrivateStateThreadIsValid())
+ {
while (!receipt_received)
{
bool timed_out = false;
@@ -4137,23 +4139,24 @@
if (!receipt_received)
{
// Check if the private state thread is still around. If it isn't then we are done waiting
- if (!m_private_state_thread.IsJoinable())
- break; // Private state thread exited, we are done
+ if (!PrivateStateThreadIsValid())
+ break; // Private state thread exited or is exiting, we are done
}
}
-
- if (signal == eBroadcastInternalStateControlStop)
- {
- thread_result_t result = NULL;
- private_state_thread.Join(&result);
- }
}
- else
+
+ if (signal == eBroadcastInternalStateControlStop)
{
- if (log)
- log->Printf ("Private state thread already dead, no need to signal it to stop.");
+ thread_result_t result = NULL;
+ m_private_state_thread.Join(&result);
+ m_private_state_thread.Reset();
}
}
+ else
+ {
+ if (log)
+ log->Printf("Private state thread already dead, no need to signal it to stop.");
+ }
}
void
@@ -4446,7 +4449,6 @@
// try to change it on the way out.
if (!is_secondary_thread)
m_public_run_lock.SetStopped();
- m_private_state_thread.Reset();
return NULL;
}
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -3309,9 +3309,13 @@
bool
PrivateStateThreadIsValid () const
{
- return m_private_state_thread.IsJoinable();
+ lldb::StateType state = m_private_state.GetValue();
+ return state != lldb::eStateInvalid &&
+ state != lldb::eStateDetached &&
+ state != lldb::eStateExited &&
+ m_private_state_thread.IsJoinable();
}
-
+
void
ForceNextEventDelivery()
{
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
