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