[Lldb-commits] [lldb] r181061 - Clear up any deadlocks on Apple builds that were due to the lldb_private::Process.m_private_run_lock variable.

[Malea, Daniel]

I tried what you recommended Greg, and it seems to cause no deadlocks on
Linux if I enable the __APPLE__ ifdef'd code!

Thanks a bunch! I will re-enable the tests as soon as the process
stop-state related regressions are sorted out. For the time being, I'll
run the tests through Parallel Inspector to see if there's any other
issues the automated tool picks up.


Cheers,
Dan

On 2013-05-03 6:25 PM, "Greg Clayton" <gclayton at apple.com> wrote:

>Author: gclayton
>Date: Fri May  3 17:25:56 2013
>New Revision: 181061
>
>URL: http://llvm.org/viewvc/llvm-project?rev=181061&view=rev
>Log:
>Clear up any deadlocks on Apple builds that were due to the
>lldb_private::Process.m_private_run_lock variable.
>
>If someone on Linux and/or FreeBSD can try to comment out the " #if
>defined(__APPLE__)" that surrounds access to "m_private_run_lock" and run
>the test suite, that would be nice. The new location where the
>locking/unlocking happens is bulletproof on MacOSX, and I want to verify
>that it is good on linux as well.
>
>
>
>Modified:
>    lldb/trunk/source/Target/Process.cpp
>
>Modified: lldb/trunk/source/Target/Process.cpp
>URL: 
>http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?r
>ev=181061&r1=181060&r2=181061&view=diff
>==========================================================================
>====
>--- lldb/trunk/source/Target/Process.cpp (original)
>+++ lldb/trunk/source/Target/Process.cpp Fri May  3 17:25:56 2013
>@@ -1161,8 +1161,10 @@ Process::Finalize()
>     // contain events that have ProcessSP values in them which can keep
>this
>     // process around forever. These events need to be cleared out.
>     m_private_state_listener.Clear();
>+    m_public_run_lock.WriteTryLock(); // This will do nothing if already
>locked
>     m_public_run_lock.WriteUnlock();
> #if defined(__APPLE__)
>+    m_private_run_lock.WriteTryLock(); // This will do nothing if
>already locked
>     m_private_run_lock.WriteUnlock();
> #endif
>     m_finalize_called = true;
>@@ -1704,16 +1706,18 @@ Process::SetPrivateState (StateType new_
>     // the private process state with another run lock. Right now it
>doesn't
>     // seem like we need to do this, but if we ever do, we can uncomment
>and
>     // use this code.
>-//    const bool old_state_is_stopped = StateIsStoppedState(old_state,
>false);
>-//    const bool new_state_is_stopped = StateIsStoppedState(new_state,
>false);
>-//    if (old_state_is_stopped != new_state_is_stopped)
>-//    {
>-//        if (new_state_is_stopped)
>-//            m_private_run_lock.WriteUnlock();
>-//        else
>-//            m_private_run_lock.WriteLock();
>-//    }
>-
>+#if defined(__APPLE__)
>+    const bool old_state_is_stopped = StateIsStoppedState(old_state,
>false);
>+    const bool new_state_is_stopped = StateIsStoppedState(new_state,
>false);
>+    if (old_state_is_stopped != new_state_is_stopped)
>+    {
>+        if (new_state_is_stopped)
>+            m_private_run_lock.WriteUnlock();
>+        else
>+            m_private_run_lock.WriteLock();
>+    }
>+#endif
>+    
>     if (state_changed)
>     {
>         m_private_state.SetValueNoLock (new_state);
>@@ -3282,9 +3286,6 @@ Process::PrivateResume ()
>             else
>             {
>                 m_mod_id.BumpResumeID();
>-#if defined(__APPLE__)
>-                m_private_run_lock.WriteLock();
>-#endif
>                 error = DoResume();
>                 if (error.Success())
>                 {
>@@ -3293,12 +3294,6 @@ Process::PrivateResume ()
>                     if (log)
>                         log->Printf ("Process thinks the process has
>resumed.");
>                 }
>-#if defined(__APPLE__)
>-                else
>-                {
>-                    m_private_run_lock.WriteUnlock();
>-                }
>-#endif
>             }
>         }
>         else
>@@ -3670,10 +3665,6 @@ Process::ShouldBroadcastEvent (Event *ev
>             // If we are going to stop, then we always broadcast the
>event.
>             // If we aren't going to stop, let the thread plans decide
>if we're going to report this event.
>             // If no thread has an opinion, we don't report it.
>-            
>-#if defined(__APPLE__)
>-            m_private_run_lock.WriteUnlock();
>-#endif
>             RefreshStateAfterStop ();
>             if (ProcessEventData::GetInterruptedFromEvent (event_ptr))
>             {
>
>
>_______________________________________________
>lldb-commits mailing list
>lldb-commits at cs.uiuc.edu
>http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits