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.
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
Editline.cpp/.h : Editline::Hide(), and other functions use m_getting_line from eventhandler thread
EditLine::GetLine() called from main thread via IOHandlerEditLine::Run()
IOHandler::Activate(), Deactivate() accesses m_active, called in Debugger::ExecuteIOHandlers() in main thread,
Debugger::PopIOHandler() can also call those functions from event handler thread
IOHandlerProcessSTDIO::OpenPipes() will read/write to m_pipe from main thread
IOHandlerProcessSTDIO::Cancel() will call m_pipe.Write() from event handler thread
Process.h: m_private_state_thread - accessed from multiple places from main thread
and from internal state thread:
Process::RunPrivateStateThread() clears this value when exiting. I don’t think this is safe in all possible cases
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.
m_private_state_control_wait.SetValue (true, eBroadcastAlways);
called in private state thread
In ~Process(), if m_private_state_thread null, it will not call ControlPrivateStateThread() to shut the thread down.
Similar to point 5:
ProcessGDBRemote::AsyncThread(), can clear m_async_thread when shutting down, while it is access many places from the main thread.
Communication::m_read_thread_enabled is read from ReadThread(), but modified from main()
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