[Lldb-commits] [lldb] r225297 - Remove a lock acquisition from ProcessWindows::OnExitProcess.

[Zachary Turner]

Author: zturner
Date: Tue Jan  6 14:56:12 2015
New Revision: 225297

URL: http://llvm.org/viewvc/llvm-project?rev=225297&view=rev
Log:
Remove a lock acquisition from ProcessWindows::OnExitProcess.

This was causing a race condition where DoDestroy() would acquire
the lock and then initiate a shutdown and then wait for it to
complete.  But part of the shutdown involved acquiring the same
lock from a different thread.  So the main thread would timeout
waiting for the shutdown to complete and return too soon.

The end result of this is that SBProcess::Kill() was broken on
Windows.

Modified:
    lldb/trunk/source/Plugins/Process/Windows/ProcessWindows.cpp

Modified: lldb/trunk/source/Plugins/Process/Windows/ProcessWindows.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/ProcessWindows.cpp?rev=225297&r1=225296&r2=225297&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Windows/ProcessWindows.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Windows/ProcessWindows.cpp Tue Jan  6 14:56:12 2015
@@ -196,6 +196,10 @@ Error
 ProcessWindows::DoLaunch(Module *exe_module,
                          ProcessLaunchInfo &launch_info)
 {
+    // Even though m_session_data is accessed here, it is before a debugger thread has been
+    // kicked off.  So there's no race conditions, and it shouldn't be necessary to acquire
+    // the mutex.
+
     Error result;
     if (!launch_info.GetFlags().Test(eLaunchFlagDebug))
     {
@@ -367,6 +371,7 @@ ProcessWindows::DoHalt(bool &caused_stop
         caused_stop = false;
     else
     {
+        llvm::sys::ScopedLock lock(m_mutex);
         caused_stop = ::DebugBreakProcess(m_session_data->m_debugger->GetProcess().GetNativeProcess().GetSystemHandle());
         if (!caused_stop)
             error.SetError(GetLastError(), eErrorTypeWin32);
@@ -391,11 +396,11 @@ ProcessWindows::DoReadMemory(lldb::addr_
                              size_t size,
                              Error &error)
 {
+    llvm::sys::ScopedLock lock(m_mutex);
+
     if (!m_session_data)
         return 0;
 
-    llvm::sys::ScopedLock lock(m_mutex);
-
     HostProcess process = m_session_data->m_debugger->GetProcess();
     void *addr = reinterpret_cast<void *>(vm_addr);
     SIZE_T bytes_read = 0;
@@ -407,11 +412,11 @@ ProcessWindows::DoReadMemory(lldb::addr_
 size_t
 ProcessWindows::DoWriteMemory(lldb::addr_t vm_addr, const void *buf, size_t size, Error &error)
 {
+    llvm::sys::ScopedLock lock(m_mutex);
+
     if (!m_session_data)
         return 0;
 
-    llvm::sys::ScopedLock lock(m_mutex);
-
     HostProcess process = m_session_data->m_debugger->GetProcess();
     void *addr = reinterpret_cast<void *>(vm_addr);
     SIZE_T bytes_written = 0;
@@ -451,7 +456,7 @@ ProcessWindows::CanDebug(Target &target,
 void
 ProcessWindows::OnExitProcess(uint32_t exit_code)
 {
-    llvm::sys::ScopedLock lock(m_mutex);
+    // No need to acquire the lock since m_session_data isn't accessed.
 
     ModuleSP executable_module = GetTarget().GetExecutableModule();
     ModuleList unloaded_modules;
@@ -467,8 +472,6 @@ ProcessWindows::OnDebuggerConnected(lldb
 {
     // Either we successfully attached to an existing process, or we successfully launched a new
     // process under the debugger.
-    llvm::sys::ScopedLock lock(m_mutex);
-
     ModuleSP module = GetTarget().GetExecutableModule();
     bool load_addr_changed;
     module->SetLoadAddress(GetTarget(), image_base, false, load_addr_changed);
@@ -479,6 +482,8 @@ ProcessWindows::OnDebuggerConnected(lldb
     loaded_modules.Append(module);
     GetTarget().ModulesDidLoad(loaded_modules);
 
+    llvm::sys::ScopedLock lock(m_mutex);
+
     DebuggerThreadSP debugger = m_session_data->m_debugger;
     const HostThreadWindows &wmain_thread = debugger->GetMainThread().GetNativeThread();
     m_session_data->m_new_threads[wmain_thread.GetThreadId()] = debugger->GetMainThread();
@@ -566,8 +571,6 @@ ProcessWindows::OnExitThread(const HostT
 void
 ProcessWindows::OnLoadDll(const ModuleSpec &module_spec, lldb::addr_t module_addr)
 {
-    llvm::sys::ScopedLock lock(m_mutex);
-
     // Confusingly, there is no Target::AddSharedModule.  Instead, calling GetSharedModule() with
     // a new module will add it to the module list and return a corresponding ModuleSP.
     Error error;
@@ -583,8 +586,6 @@ ProcessWindows::OnLoadDll(const ModuleSp
 void
 ProcessWindows::OnUnloadDll(lldb::addr_t module_addr)
 {
-    llvm::sys::ScopedLock lock(m_mutex);
-
     Address resolved_addr;
     if (GetTarget().ResolveLoadAddress(module_addr, resolved_addr))
     {