<div dir="ltr">Thanks!</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 1, 2014 at 6:04 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> + // Validate we know about the deferred trigger thread.<br>
> + if (auto find_it = coordinator.m_tid_stop_map.find (m_triggering_tid) == coordinator.m_tid_stop_map.end ())<br>
> + {<br>
<br>
</span>find_it is unused here :)<br>
<br>
I've removed it in r218844.<br>
<span class="HOEnZb"><font color="#888888"><br>
-eric<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> + // We don't know about this thread. This is an error condition.<br>
> + std::ostringstream error_message;<br>
> + error_message << "error: deferred notification tid " << m_triggering_tid << " is unknown";<br>
> + m_error_function (error_message.str ());<br>
> +<br>
> + // We bail out here.<br>
> + return eventLoopResultContinue;<br>
> + }<br>
> +<br>
> // Request a stop for all the thread stops that need to be stopped<br>
> // and are not already known to be stopped. Keep a list of all the<br>
> // threads from which we still need to hear a stop reply.<br>
> @@ -108,10 +123,24 @@ public:<br>
> ThreadIDSet sent_tids;<br>
> for (auto tid : m_wait_for_stop_tids)<br>
> {<br>
> - // If we don't know about the thread's stop state or we<br>
> - // know it is not stopped, we need to send it a stop request.<br>
> + // Validate we know about all tids for which we must first receive a stop before<br>
> + // triggering the deferred stop notification.<br>
> auto find_it = coordinator.m_tid_stop_map.find (tid);<br>
> - if ((find_it == coordinator.m_tid_stop_map.end ()) || !find_it->second)<br>
> + if (find_it == coordinator.m_tid_stop_map.end ())<br>
> + {<br>
> + // This is an error. We shouldn't be asking for waiting pids that aren't known.<br>
> + // NOTE: we may be stripping out the specification of wait tids and handle this<br>
> + // automatically, in which case this state can never occur.<br>
> + std::ostringstream error_message;<br>
> + error_message << "error: deferred notification for tid " << m_triggering_tid << " specified an unknown/untracked pending stop tid " << m_triggering_tid;<br>
> + m_error_function (error_message.str ());<br>
> +<br>
> + // Bail out here.<br>
> + return eventLoopResultContinue;<br>
> + }<br>
> +<br>
> + // If the pending stop thread is currently running, we need to send it a stop request.<br>
> + if (!find_it->second)<br>
> {<br>
> m_request_thread_stop_function (tid);<br>
> sent_tids.insert (tid);<br>
> @@ -137,7 +166,7 @@ public:<br>
> coordinator.SetPendingNotification (shared_from_this ());<br>
> }<br>
><br>
> - return true;<br>
> + return eventLoopResultContinue;<br>
> }<br>
><br>
> // Return true if still pending thread stops waiting; false if no more stops.<br>
> @@ -184,6 +213,7 @@ private:<br>
> const ThreadIDSet m_original_wait_for_stop_tids;<br>
> ThreadIDFunction m_request_thread_stop_function;<br>
> ThreadIDFunction m_call_after_function;<br>
> + ErrorFunction m_error_function;<br>
> };<br>
><br>
> //===----------------------------------------------------------------------===//<br>
> @@ -196,11 +226,11 @@ public:<br>
> {<br>
> }<br>
><br>
> - bool<br>
> + EventLoopResult<br>
> ProcessEvent(ThreadStateCoordinator &coordinator) override<br>
> {<br>
> coordinator.ResetNow ();<br>
> - return true;<br>
> + return eventLoopResultContinue;<br>
> }<br>
> };<br>
><br>
> @@ -209,22 +239,25 @@ public:<br>
> class ThreadStateCoordinator::EventThreadStopped : public ThreadStateCoordinator::EventBase<br>
> {<br>
> public:<br>
> - EventThreadStopped (lldb::tid_t tid):<br>
> + EventThreadStopped (lldb::tid_t tid,<br>
> + const ErrorFunction &error_function):<br>
> EventBase (),<br>
> - m_tid (tid)<br>
> + m_tid (tid),<br>
> + m_error_function (error_function)<br>
> {<br>
> }<br>
><br>
> - bool<br>
> + EventLoopResult<br>
> ProcessEvent(ThreadStateCoordinator &coordinator) override<br>
> {<br>
> - coordinator.ThreadDidStop (m_tid);<br>
> - return true;<br>
> + coordinator.ThreadDidStop (m_tid, m_error_function);<br>
> + return eventLoopResultContinue;<br>
> }<br>
><br>
> private:<br>
><br>
> const lldb::tid_t m_tid;<br>
> + ErrorFunction m_error_function;<br>
> };<br>
><br>
> //===----------------------------------------------------------------------===//<br>
> @@ -232,22 +265,28 @@ private:<br>
> class ThreadStateCoordinator::EventThreadCreate : public ThreadStateCoordinator::EventBase<br>
> {<br>
> public:<br>
> - EventThreadCreate (lldb::tid_t tid):<br>
> + EventThreadCreate (lldb::tid_t tid,<br>
> + bool is_stopped,<br>
> + const ErrorFunction &error_function):<br>
> EventBase (),<br>
> - m_tid (tid)<br>
> + m_tid (tid),<br>
> + m_is_stopped (is_stopped),<br>
> + m_error_function (error_function)<br>
> {<br>
> }<br>
><br>
> - bool<br>
> + EventLoopResult<br>
> ProcessEvent(ThreadStateCoordinator &coordinator) override<br>
> {<br>
> - coordinator.ThreadWasCreated (m_tid);<br>
> - return true;<br>
> + coordinator.ThreadWasCreated (m_tid, m_is_stopped);<br>
> + return eventLoopResultContinue;<br>
> }<br>
><br>
> private:<br>
><br>
> const lldb::tid_t m_tid;<br>
> + const bool m_is_stopped;<br>
> + ErrorFunction m_error_function;<br>
> };<br>
><br>
> //===----------------------------------------------------------------------===//<br>
> @@ -255,22 +294,25 @@ private:<br>
> class ThreadStateCoordinator::EventThreadDeath : public ThreadStateCoordinator::EventBase<br>
> {<br>
> public:<br>
> - EventThreadDeath (lldb::tid_t tid):<br>
> + EventThreadDeath (lldb::tid_t tid,<br>
> + const ErrorFunction &error_function):<br>
> EventBase (),<br>
> - m_tid (tid)<br>
> + m_tid (tid),<br>
> + m_error_function (error_function)<br>
> {<br>
> }<br>
><br>
> - bool<br>
> + EventLoopResult<br>
> ProcessEvent(ThreadStateCoordinator &coordinator) override<br>
> {<br>
> coordinator.ThreadDidDie (m_tid);<br>
> - return true;<br>
> + return eventLoopResultContinue;<br>
> }<br>
><br>
> private:<br>
><br>
> const lldb::tid_t m_tid;<br>
> + ErrorFunction m_error_function;<br>
> };<br>
><br>
> //===----------------------------------------------------------------------===//<br>
> @@ -278,14 +320,17 @@ private:<br>
> class ThreadStateCoordinator::EventRequestResume : public ThreadStateCoordinator::EventBase<br>
> {<br>
> public:<br>
> - EventRequestResume (lldb::tid_t tid, const ThreadIDFunction &request_thread_resume_function):<br>
> + EventRequestResume (lldb::tid_t tid,<br>
> + const ThreadIDFunction &request_thread_resume_function,<br>
> + const ErrorFunction &error_function):<br>
> EventBase (),<br>
> m_tid (tid),<br>
> - m_request_thread_resume_function (request_thread_resume_function)<br>
> + m_request_thread_resume_function (request_thread_resume_function),<br>
> + m_error_function (error_function)<br>
> {<br>
> }<br>
><br>
> - bool<br>
> + EventLoopResult<br>
> ProcessEvent(ThreadStateCoordinator &coordinator) override<br>
> {<br>
> // Tell the thread to resume if we don't already think it is running.<br>
> @@ -294,13 +339,13 @@ public:<br>
> {<br>
> // Skip the resume call - we think it is already running because we don't know anything about the thread.<br>
> coordinator.Log ("EventRequestResume::%s skipping resume request because we don't know about tid %" PRIu64 " and we therefore assume it is running.", __FUNCTION__, m_tid);<br>
> - return true;<br>
> + return eventLoopResultContinue;<br>
> }<br>
> else if (!find_it->second)<br>
> {<br>
> // Skip the resume call - we have tracked it to be running.<br>
> coordinator.Log ("EventRequestResume::%s skipping resume request because tid %" PRIu64 " is already running according to our state tracking.", __FUNCTION__, m_tid);<br>
> - return true;<br>
> + return eventLoopResultContinue;<br>
> }<br>
><br>
> // Before we do the resume below, first check if we have a pending<br>
> @@ -333,13 +378,14 @@ public:<br>
> // to reflect it is running after this completes.<br>
> m_request_thread_resume_function (m_tid);<br>
><br>
> - return true;<br>
> + return eventLoopResultContinue;<br>
> }<br>
><br>
> private:<br>
><br>
> const lldb::tid_t m_tid;<br>
> ThreadIDFunction m_request_thread_resume_function;<br>
> + ErrorFunction m_error_function;<br>
> };<br>
><br>
> //===----------------------------------------------------------------------===//<br>
> @@ -403,19 +449,32 @@ void<br>
> ThreadStateCoordinator::CallAfterThreadsStop (const lldb::tid_t triggering_tid,<br>
> const ThreadIDSet &wait_for_stop_tids,<br>
> const ThreadIDFunction &request_thread_stop_function,<br>
> - const ThreadIDFunction &call_after_function)<br>
> + const ThreadIDFunction &call_after_function,<br>
> + const ErrorFunction &error_function)<br>
> {<br>
> EnqueueEvent (EventBaseSP (new EventCallAfterThreadsStop (triggering_tid,<br>
> wait_for_stop_tids,<br>
> request_thread_stop_function,<br>
> - call_after_function)));<br>
> + call_after_function,<br>
> + error_function)));<br>
> }<br>
><br>
> void<br>
> -ThreadStateCoordinator::ThreadDidStop (lldb::tid_t tid)<br>
> +ThreadStateCoordinator::ThreadDidStop (lldb::tid_t tid, ErrorFunction &error_function)<br>
> {<br>
> + // Ensure we know about the thread.<br>
> + auto find_it = m_tid_stop_map.find (tid);<br>
> + if (find_it == m_tid_stop_map.end ())<br>
> + {<br>
> + // We don't know about this thread. This is an error condition.<br>
> + std::ostringstream error_message;<br>
> + error_message << "error: tid " << tid << " asked to stop but tid is unknown";<br>
> + error_function (error_message.str ());<br>
> + return;<br>
> + }<br>
> +<br>
> // Update the global list of known thread states. This one is definitely stopped.<br>
> - m_tid_stop_map[tid] = true;<br>
> + find_it->second = true;<br>
><br>
> // If we have a pending notification, remove this from the set.<br>
> EventCallAfterThreadsStop *const call_after_event = GetPendingThreadStopNotification ();<br>
> @@ -431,14 +490,13 @@ ThreadStateCoordinator::ThreadDidStop (l<br>
> }<br>
><br>
> void<br>
> -ThreadStateCoordinator::ThreadWasCreated (lldb::tid_t tid)<br>
> +ThreadStateCoordinator::ThreadWasCreated (lldb::tid_t tid, bool is_stopped)<br>
> {<br>
> // Add the new thread to the stop map.<br>
> - // We assume a created thread is not stopped.<br>
> - m_tid_stop_map[tid] = false;<br>
> + m_tid_stop_map[tid] = is_stopped;<br>
><br>
> EventCallAfterThreadsStop *const call_after_event = GetPendingThreadStopNotification ();<br>
> - if (call_after_event)<br>
> + if (call_after_event && !is_stopped)<br>
> {<br>
> // Tell the pending notification that we need to wait<br>
> // for this new thread to stop.<br>
> @@ -492,27 +550,33 @@ ThreadStateCoordinator::Log (const char<br>
> }<br>
><br>
> void<br>
> -ThreadStateCoordinator::NotifyThreadStop (lldb::tid_t tid)<br>
> +ThreadStateCoordinator::NotifyThreadStop (lldb::tid_t tid,<br>
> + const ErrorFunction &error_function)<br>
> {<br>
> - EnqueueEvent (EventBaseSP (new EventThreadStopped (tid)));<br>
> + EnqueueEvent (EventBaseSP (new EventThreadStopped (tid, error_function)));<br>
> }<br>
><br>
> void<br>
> -ThreadStateCoordinator::RequestThreadResume (lldb::tid_t tid, const ThreadIDFunction &request_thread_resume_function)<br>
> +ThreadStateCoordinator::RequestThreadResume (lldb::tid_t tid,<br>
> + const ThreadIDFunction &request_thread_resume_function,<br>
> + const ErrorFunction &error_function)<br>
> {<br>
> - EnqueueEvent (EventBaseSP (new EventRequestResume (tid, request_thread_resume_function)));<br>
> + EnqueueEvent (EventBaseSP (new EventRequestResume (tid, request_thread_resume_function, error_function)));<br>
> }<br>
><br>
> void<br>
> -ThreadStateCoordinator::NotifyThreadCreate (lldb::tid_t tid)<br>
> +ThreadStateCoordinator::NotifyThreadCreate (lldb::tid_t tid,<br>
> + bool is_stopped,<br>
> + const ErrorFunction &error_function)<br>
> {<br>
> - EnqueueEvent (EventBaseSP (new EventThreadCreate (tid)));<br>
> + EnqueueEvent (EventBaseSP (new EventThreadCreate (tid, is_stopped, error_function)));<br>
> }<br>
><br>
> void<br>
> -ThreadStateCoordinator::NotifyThreadDeath (lldb::tid_t tid)<br>
> +ThreadStateCoordinator::NotifyThreadDeath (lldb::tid_t tid,<br>
> + const ErrorFunction &error_function)<br>
> {<br>
> - EnqueueEvent (EventBaseSP (new EventThreadDeath (tid)));<br>
> + EnqueueEvent (EventBaseSP (new EventThreadDeath (tid, error_function)));<br>
> }<br>
><br>
> void<br>
> @@ -541,7 +605,7 @@ ThreadStateCoordinator::StopCoordinator<br>
> EnqueueEvent (EventBaseSP (new EventStopCoordinator ()));<br>
> }<br>
><br>
> -bool<br>
> +ThreadStateCoordinator::EventLoopResult<br>
> ThreadStateCoordinator::ProcessNextEvent ()<br>
> {<br>
> return DequeueEventWithWait()->ProcessEvent (*this);<br>
><br>
> Modified: lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h?rev=218833&r1=218832&r2=218833&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h?rev=218833&r1=218832&r2=218833&view=diff</a><br>
> ==============================================================================<br>
> --- lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h (original)<br>
> +++ lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h Wed Oct 1 16:40:45 2014<br>
> @@ -28,36 +28,62 @@ namespace lldb_private<br>
> // Typedefs.<br>
> typedef std::unordered_set<lldb::tid_t> ThreadIDSet;<br>
><br>
> - // Protocols.<br>
> -<br>
> + enum EventLoopResult<br>
> + {<br>
> + eventLoopResultContinue,<br>
> + eventLoopResultStop<br>
> + };<br>
><br>
> // Callback/block definitions.<br>
> typedef std::function<void (lldb::tid_t tid)> ThreadIDFunction;<br>
> typedef std::function<void (const char *format, va_list args)> LogFunction;<br>
> + typedef std::function<void (const std::string &error_message)> ErrorFunction;<br>
><br>
> - // constructors<br>
> + // Constructors.<br>
> ThreadStateCoordinator (const LogFunction &log_function);<br>
><br>
> - // The main purpose of the class: triggering an action after<br>
> - // a given set of threads stop.<br>
> + // Notify the coordinator when a thread is created and/or starting to be<br>
> + // tracked. is_stopped should be true if the thread is currently stopped;<br>
> + // otherwise, it should be set false if it is already running. Will<br>
> + // call the error function if the thread id is already tracked.<br>
> + void<br>
> + NotifyThreadCreate (lldb::tid_t tid,<br>
> + bool is_stopped,<br>
> + const ErrorFunction &error_function);<br>
> +<br>
> + // Notify the coordinator when a previously-existing thread should no<br>
> + // longer be tracked. The error_function will trigger if the thread<br>
> + // is not being tracked.<br>
> + void<br>
> + NotifyThreadDeath (lldb::tid_t tid,<br>
> + const ErrorFunction &error_function);<br>
> +<br>
> +<br>
> + // This method is the main purpose of the class: triggering a deferred<br>
> + // action after a given set of threads stop. The triggering_tid is the<br>
> + // thread id passed to the call_after_function. The error_function will<br>
> + // be fired if either the triggering tid or any of the wait_for_stop_tids<br>
> + // are unknown at the time the method is processed.<br>
> void<br>
> CallAfterThreadsStop (lldb::tid_t triggering_tid,<br>
> const ThreadIDSet &wait_for_stop_tids,<br>
> const ThreadIDFunction &request_thread_stop_function,<br>
> - const ThreadIDFunction &call_after_function);<br>
> -<br>
> - // Notifications called when various state changes occur.<br>
> - void<br>
> - NotifyThreadStop (lldb::tid_t tid);<br>
> + const ThreadIDFunction &call_after_function,<br>
> + const ErrorFunction &error_function);<br>
><br>
> + // Notify the thread stopped. Will trigger error at time of execution if we<br>
> + // already think it is stopped.<br>
> void<br>
> - RequestThreadResume (lldb::tid_t tid, const ThreadIDFunction &request_thread_resume_func);<br>
> + NotifyThreadStop (lldb::tid_t tid,<br>
> + const ErrorFunction &error_function);<br>
><br>
> - void<br>
> - NotifyThreadCreate (lldb::tid_t tid);<br>
> -<br>
> - void<br>
> - NotifyThreadDeath (lldb::tid_t tid);<br>
> + // Request that the given thread id should have the request_thread_resume_function<br>
> + // called. Will trigger the error_function if the thread is thought to be running<br>
> + // already at that point.<br>
> + void<br>
> + RequestThreadResume (lldb::tid_t tid,<br>
> + const ThreadIDFunction &request_thread_resume_function,<br>
> + const ErrorFunction &error_function);<br>
><br>
> // Indicate the calling process did an exec and that the thread state<br>
> // should be 100% cleared.<br>
> @@ -76,7 +102,7 @@ namespace lldb_private<br>
> // Expected usage is to run this in a separate thread until the function<br>
> // returns false. Always call this from the same thread. The processing<br>
> // logic assumes the execution of this is implicitly serialized.<br>
> - bool<br>
> + EventLoopResult<br>
> ProcessNextEvent ();<br>
><br>
> private:<br>
> @@ -111,10 +137,10 @@ namespace lldb_private<br>
> SetPendingNotification (const EventBaseSP &event_sp);<br>
><br>
> void<br>
> - ThreadDidStop (lldb::tid_t tid);<br>
> + ThreadDidStop (lldb::tid_t tid, ErrorFunction &error_function);<br>
><br>
> void<br>
> - ThreadWasCreated (lldb::tid_t tid);<br>
> + ThreadWasCreated (lldb::tid_t tid, bool is_stopped);<br>
><br>
> void<br>
> ThreadDidDie (lldb::tid_t tid);<br>
><br>
><br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><table cellspacing="0" cellpadding="0" style="color:rgb(136,136,136);font-family:'Times New Roman'"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Todd Fiala |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tfiala@google.com" style="color:rgb(17,85,204)" target="_blank"><span style="background-color:rgb(255,255,204);color:rgb(34,34,34);background-repeat:initial initial">tfiala@google.com</span></a></td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"><br></td></tr></tbody></table><br></div>
</div>