<div dir="ltr">Hey Alexander,<div><br></div><div>We totally agree. We're in the process of getting internal build bots that look for this. We've been working with Chandler on getting an externally-visible buildbot infrastructure up.</div><div><br></div><div>For the time being, there is a lot of noise in the tsan reports. Since we won't be able to just focus on squelching all of those at once, we're likely going to try to filter the tsan output to have it indicate new occurrences, and get to fixing the more important ones as we find them.</div><div><br></div><div>That's the long-term plan, at least.</div><div><br></div><div>-Todd</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 19, 2014 at 7:09 AM, Alexander Potapenko <span dir="ltr"><<a href="mailto:glider@google.com" target="_blank">glider@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Shawn,<br>
<br>
This is really great that you've committed your time to fix some these<br>
issues. But our experience shows that without regular testing new bugs<br>
will appear in the codebase soon.<br>
Perhaps a good idea is to add a buildbot running lldb tests under<br>
ThreadSanitizer, if the lldb team is interested in investing into<br>
that.<br>
<br>
Regarding the race reports that are "not an issue in practice", such<br>
cases still violate the C++ Standard, so nobody knows when they're<br>
going to break.<br>
<br>
HTH,<br>
Alex.<br>
<div><div class="h5"><br>
On Tue, Sep 16, 2014 at 5:39 AM, Shawn Best <<a href="mailto:sbest@blueshiftinc.com">sbest@blueshiftinc.com</a>> wrote:<br>
> Hi,<br>
><br>
><br>
> I have recently built lldb with an automated tool “ThreadSanitizer( or<br>
> tsan)” designed to detect race conditions and similar concurrency problems.<br>
> It turned up a number of potential issues. Some may be real, others are<br>
> probably not an issue in practice. I submitted a patch which fixed a few,<br>
> either using a mutex, or std::atomic< >. The process of manually analyzing<br>
> each warning involves a lot of time consuming legwork. In many cases, the<br>
> work to quiet the warning would add code complexity and runtime hit that may<br>
> not be worth it.<br>
><br>
><br>
> I need to back-burner this for a bit and move on tow work on something<br>
> different. I thought I would post some of my findings here as there may<br>
> still be some real bugs in there.<br>
><br>
><br>
> 1. ==========================<br>
><br>
><br>
> Timer.cpp: g_depth : this is a class static variable used in constructor and<br>
> destructor. Use of a timer object from different threads will have a race.<br>
><br>
><br>
> CommandInterpreter::HandleCommand() uses Timer scopedTimer; // called from<br>
> main<br>
><br>
><br>
> Disassembler::FindPlugin() uses scoped_timer, called from event handler<br>
> thread<br>
><br>
><br>
> 2. ===========================<br>
><br>
><br>
> Editline.cpp/.h : Editline::Hide(), and other functions use m_getting_line<br>
> from eventhandler thread<br>
><br>
><br>
> EditLine::GetLine() called from main thread via IOHandlerEditLine::Run()<br>
><br>
><br>
> 3. ===========================<br>
><br>
><br>
> IOHandler::Activate(), Deactivate() accesses m_active, called in<br>
> Debugger::ExecuteIOHandlers() in main thread,<br>
><br>
> Debugger::PopIOHandler() can also call those functions from event handler<br>
> thread<br>
><br>
><br>
> 4. ===================<br>
><br>
><br>
> IOHandlerProcessSTDIO::OpenPipes() will read/write to m_pipe from main<br>
> thread<br>
><br>
> IOHandlerProcessSTDIO::Cancel() will call m_pipe.Write() from event handler<br>
> thread<br>
><br>
><br>
> 5. =======================<br>
><br>
> Process.h: m_private_state_thread - accessed from multiple places from main<br>
> thread<br>
><br>
> Process::RunThreadPlan(),<br>
><br>
> Process::StartPrivateStateThread(),<br>
><br>
> Process::ControlPrivateStateThread()<br>
><br>
> Process::PrivateStateThreadIsValid()<br>
><br>
><br>
> and from internal state thread:<br>
><br>
> Process::RunPrivateStateThread() clears this value when exiting. I don’t<br>
> think this is safe in all possible cases<br>
><br>
><br>
> 6. ===========================<br>
><br>
><br>
> There is a whole class of problems related to resources created in Process,<br>
> and used by multiple threads. TSAN complains when the ~Process() destructor<br>
> is called and destroys those resources. From what I can tell, the threads<br>
> are typically shut down and not using those resources at that point, but if<br>
> a thread were to shut itself down asynchronously, I think it is possible for<br>
> the resources to get destroyed without a formal interlock on those events.<br>
><br>
><br>
> Things like:<br>
><br>
> m_public_run_lock.SetStopped();<br>
><br>
> m_private_state_control_wait.SetValue (true, eBroadcastAlways);<br>
><br>
> m_private_state_thread.Reset();<br>
><br>
> called in private state thread<br>
><br>
><br>
> In ~Process(), if m_private_state_thread null, it will not call<br>
> ControlPrivateStateThread() to shut the thread down.<br>
><br>
><br>
> 7. ============================<br>
><br>
> Similar to point 5:<br>
><br>
><br>
> ProcessGDBRemote::AsyncThread(), can clear m_async_thread when shutting<br>
> down, while it is access many places from the main thread.<br>
><br>
><br>
> 8. ===========================<br>
><br>
> Communication::m_read_thread_enabled is read from ReadThread(), but<br>
> modified from main()<br>
><br>
><br>
> 9. ============================<br>
><br>
><br>
> ProcessGDBRemote::DoDestroy() will call a Socket::Close() from main thread.<br>
><br>
><br>
> An error condition in reading a socket in the async thread will call<br>
> Disconnect(), accessing the same socket variable<br>
><br>
><br>
><br>
</div></div>> _______________________________________________<br>
> lldb-dev mailing list<br>
> <a href="mailto:lldb-dev@cs.uiuc.edu">lldb-dev@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</a><br>
><br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
<br>
--<br>
Alexander Potapenko<br>
Software Engineer<br>
Google Moscow<br>
<br>
_______________________________________________<br>
lldb-dev mailing list<br>
<a href="mailto:lldb-dev@cs.uiuc.edu">lldb-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</a><br>
</font></span></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"><font color="#1155cc"> <a>650-943-3180</a></font></td></tr></tbody></table><br></div>
</div>