Permalink
Browse files

Use flag instead of lock state to track running status

LLDB requires that the inferior process be stopped before, and remain
stopped during, certain accesses to process state.

Previously this was achieved with a wrlock that was held while the process
was running, and released while the process was stopped.  Any access to
process state was performed with a read lock held.

POSIX requires that pthread_rwlock_unlock() be called from the same thread
as pthread_rwlock_wrlock(), but lldb needs to stop and start the process
from different threads.

All read lock consumers use ReadTryLock() and handle failure to obtain the
lock (typically by logging an error "process is running").  Thus, instead
of using the lock state itself to track the running state, add an explicit
m_running flag.  ReadTryLock tests the flag, and keeps the read lock held
if available.  WriteLock and WriteTryLock (if successful) set m_running
with the lock held.  This way, read consumers can determine if the process
is running and act appropriately, and write consumers are still held off
from starting the process if read consumers are active.

Note that with this change there are still some questionable access
patterns, such as calling WriteUnlock twice in a row, and there's no
protection from multiple threads trying to simultaneously start the
process (i.e., take the write lock).  In practice this does not seem to be
a problem, and was already exposing undefined POSIX behaviour.

This is a proof of concept patch, and should be cleaned up further
assuming the approach is sound -- at least, ReadLock / WriteLock should be
renamed.
  • Loading branch information...
1 parent e89ccd7 commit 9566f404a35274d68cd29a183cff80cbe864bff7 @emaste emaste committed Jul 2, 2013
Showing with 25 additions and 5 deletions.
  1. +25 −5 include/lldb/Host/ReadWriteLock.h
@@ -37,7 +37,8 @@ class ReadWriteLock
{
public:
ReadWriteLock () :
- m_rwlock()
+ m_rwlock(),
+ m_running(false)
{
int err = ::pthread_rwlock_init(&m_rwlock, NULL); (void)err;
//#if LLDB_CONFIGURATION_DEBUG
@@ -56,7 +57,13 @@ class ReadWriteLock
bool
ReadTryLock ()
{
- return ::pthread_rwlock_tryrdlock (&m_rwlock) == 0;
+ ::pthread_rwlock_rdlock (&m_rwlock);
+ if (m_running == false)
+ {
+ return true;
+ }
+ ::pthread_rwlock_unlock (&m_rwlock);
+ return false;
}
bool
@@ -68,19 +75,31 @@ class ReadWriteLock
bool
WriteLock()
{
- return ::pthread_rwlock_wrlock (&m_rwlock) == 0;
+ ::pthread_rwlock_wrlock (&m_rwlock);
+ m_running = true;
+ ::pthread_rwlock_unlock (&m_rwlock);
+ return true;
}
bool
WriteTryLock()
{
- return ::pthread_rwlock_trywrlock (&m_rwlock) == 0;
+ if (::pthread_rwlock_trywrlock (&m_rwlock) == 0)
+ {
+ m_running = true;
+ ::pthread_rwlock_unlock (&m_rwlock);
+ return true;
+ }
+ return false;
}
bool
WriteUnlock ()
{
- return ::pthread_rwlock_unlock (&m_rwlock) == 0;
+ ::pthread_rwlock_wrlock (&m_rwlock);
+ m_running = false;
+ ::pthread_rwlock_unlock (&m_rwlock);
+ return true;
}
class ReadLocker
@@ -136,6 +155,7 @@ class ReadWriteLock
protected:
pthread_rwlock_t m_rwlock;
+ bool m_running;
private:
DISALLOW_COPY_AND_ASSIGN(ReadWriteLock);
};

0 comments on commit 9566f40

Please sign in to comment.