[lldb-dev] Potential race conditions identified by ThreadSanitizer

[Shawn Best]

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_wai*t.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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140915/7958ab53/attachment.html>