[lldb-dev] Locking issues on windows

[Greg Clayton]

On Apr 17, 2013, at 1:27 AM, Carlo Kok <ck at remobjects.com> wrote:

> I'm trying to update the Windows branch to the latest and greatest and found these locking issues (not sure if they're relevant for posix too):
> 
> When I attach a process (I only use the gdb remote) the first even I get is "stopped" which tries to unlock m_private_run_lock, however this one is never locked in the first place. Windows' writelock doesn't appreciate that; as a workaround I added a
> m_private_run_lock.WriteLock(); in Process' constructor, which seems to fix that.

We need to fix this better by locking the private run lock when attaching if all goes well.

> 
> The second issue occurs when when trying to cause a "Stop" when it's already paused on internal breakpoints; for me this is during slow symbol load. When happens is that the loading (which happens from within Process::ShouldBroadcastEvent) resumes it, then the process exits properly (triggers the ShouldBroadcastEvent again) however:
> 
> ProcessEventData::DoOnRemoval(lldb_private::Event * event_ptr)
> called by Listener::FindNextEventInternal.
> 
> The resume call is in this condition:
>  if (state != eStateRunning)

Where is the above "if (state != eStateRunning)"?

> Changing that to:
> lldb::StateType state = m_process_sp->GetPrivateState();
> if (state != eStateRunning && state != eStateCrashed && state != eStateDetached && state != eStateExited)

There are functions that indicate if the function is stopped or running. We should use those functions. (search for "StateIsStopped").

> 
> Seems to fix it, as there's no reason to try & resume a process that's not running in the first place (and since exiting doesn't unlock a process this causes a deadlock)
> 
> The last issue is this:
> void * Process::RunPrivateStateThread ()
> does : m_public_run_lock.WriteUnlock(); when it's done. The Finalize also unlocks that same lock, which Windows crashes on.
> commenting that out and it seems to work stable.

We need to build in some smarts into our Read/Write locking class to know if the read/write lock is taken and only unlock if the corresponding read/write lock is locked. I will make this change today.

> 
> 
> Anyone see any issues in all of this? (might make sense to apply this to trunk too; it's never good to have unbalanced lock/unlocks)
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev