[Lldb-commits] [lldb] r187377 - Use flag instead of rwlock state to track process running state
Ed Maste
emaste at freebsd.org
Mon Jul 29 13:58:07 PDT 2013
Author: emaste
Date: Mon Jul 29 15:58:06 2013
New Revision: 187377
URL: http://llvm.org/viewvc/llvm-project?rev=187377&view=rev
Log:
Use flag instead of rwlock state to track process running state
LLDB requires that the inferior process be stopped before, and remain
stopped during, certain accesses to process state.
Previously this was achieved with a POSIX rwlock which had a write lock
taken for the duration that the process was running, and released when
the process was stopped. Any access to process state was performed with
a read lock held.
However, POSIX requires that pthread_rwlock_unlock() be called from the
same thread as pthread_rwlock_wrlock(), and lldb needs to stop and start
the process from different threads. Violating this constraint is
technically undefined behaviour, although as it happens Linux and Darwin
result in the unlock proceeding in this case. FreeBSD follows POSIX
more strictly, and the unlock would fail, resulting in a hang later upon
the next attempt to take the lock.
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, this
change adds an explicit m_running flag. ReadTryLock tests the flag, and
if the process is not running it returns with the read lock held.
WriteLock and WriteTryLock are renamed to SetRunning and TrySetRunning,
and (if successful) they 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 curious access patterns,
such as calling WriteUnlock / SetStopped twice in a row, and there's no
protection from multiple threads trying to simultaneously start the
process. In practice this does not seem to be a problem, and was
exposing other undefined POSIX behaviour prior to this change.
Added:
lldb/trunk/include/lldb/Host/ProcessRunLock.h
- copied, changed from r187376, lldb/trunk/include/lldb/Host/ReadWriteLock.h
Removed:
lldb/trunk/include/lldb/Host/ReadWriteLock.h
Modified:
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/source/Target/Process.cpp
Copied: lldb/trunk/include/lldb/Host/ProcessRunLock.h (from r187376, lldb/trunk/include/lldb/Host/ReadWriteLock.h)
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/ProcessRunLock.h?p2=lldb/trunk/include/lldb/Host/ProcessRunLock.h&p1=lldb/trunk/include/lldb/Host/ReadWriteLock.h&r1=187376&r2=187377&rev=187377&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/ReadWriteLock.h (original)
+++ lldb/trunk/include/lldb/Host/ProcessRunLock.h Mon Jul 29 15:58:06 2013
@@ -1,4 +1,4 @@
-//===-- ReadWriteLock.h -----------------------------------------*- C++ -*-===//
+//===-- ProcessRunLock.h ----------------------------------------*- C++ -*-===//
//
// The LLVM Compiler Infrastructure
//
@@ -7,8 +7,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef liblldb_ReadWriteLock_h_
-#define liblldb_ReadWriteLock_h_
+#ifndef liblldb_ProcessRunLock_h_
+#define liblldb_ProcessRunLock_h_
#if defined(__cplusplus)
#include "lldb/Host/Mutex.h"
@@ -23,21 +23,18 @@
namespace lldb_private {
//----------------------------------------------------------------------
-/// @class ReadWriteLock ReadWriteLock.h "lldb/Host/ReadWriteLock.h"
-/// @brief A C++ wrapper class for providing threaded access to a value
-/// of type T.
-///
-/// A templatized class that provides multi-threaded access to a value
-/// of type T. Threads can efficiently wait for bits within T to be set
-/// or reset, or wait for T to be set to be equal/not equal to a
-/// specified values.
+/// @class ProcessRunLock ProcessRunLock.h "lldb/Host/ProcessRunLock.h"
+/// @brief A class used to prevent the process from starting while other
+/// threads are accessing its data, and prevent access to its data while
+/// it is running.
//----------------------------------------------------------------------
-class ReadWriteLock
+class ProcessRunLock
{
public:
- ReadWriteLock () :
- m_rwlock()
+ ProcessRunLock () :
+ m_rwlock(),
+ m_running(false)
{
int err = ::pthread_rwlock_init(&m_rwlock, NULL); (void)err;
//#if LLDB_CONFIGURATION_DEBUG
@@ -45,7 +42,7 @@ public:
//#endif
}
- ~ReadWriteLock ()
+ ~ProcessRunLock ()
{
int err = ::pthread_rwlock_destroy (&m_rwlock); (void)err;
//#if LLDB_CONFIGURATION_DEBUG
@@ -56,7 +53,13 @@ public:
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
@@ -66,39 +69,54 @@ public:
}
bool
- WriteLock()
+ SetRunning()
{
- return ::pthread_rwlock_wrlock (&m_rwlock) == 0;
+ ::pthread_rwlock_wrlock (&m_rwlock);
+ m_running = true;
+ ::pthread_rwlock_unlock (&m_rwlock);
+ return true;
}
bool
- WriteTryLock()
+ TrySetRunning()
{
- return ::pthread_rwlock_trywrlock (&m_rwlock) == 0;
+ bool r;
+
+ if (::pthread_rwlock_trywrlock (&m_rwlock) == 0)
+ {
+ r = !m_running;
+ m_running = true;
+ ::pthread_rwlock_unlock (&m_rwlock);
+ return r;
+ }
+ return false;
}
bool
- WriteUnlock ()
+ SetStopped ()
{
- return ::pthread_rwlock_unlock (&m_rwlock) == 0;
+ ::pthread_rwlock_wrlock (&m_rwlock);
+ m_running = false;
+ ::pthread_rwlock_unlock (&m_rwlock);
+ return true;
}
- class ReadLocker
+ class ProcessRunLocker
{
public:
- ReadLocker () :
+ ProcessRunLocker () :
m_lock (NULL)
{
}
- ~ReadLocker()
+ ~ProcessRunLocker()
{
Unlock();
}
// Try to lock the read lock, but only do so if there are no writers.
bool
- TryLock (ReadWriteLock *lock)
+ TryLock (ProcessRunLock *lock)
{
if (m_lock)
{
@@ -129,18 +147,19 @@ public:
}
}
- ReadWriteLock *m_lock;
+ ProcessRunLock *m_lock;
private:
- DISALLOW_COPY_AND_ASSIGN(ReadLocker);
+ DISALLOW_COPY_AND_ASSIGN(ProcessRunLocker);
};
protected:
pthread_rwlock_t m_rwlock;
+ bool m_running;
private:
- DISALLOW_COPY_AND_ASSIGN(ReadWriteLock);
+ DISALLOW_COPY_AND_ASSIGN(ProcessRunLock);
};
} // namespace lldb_private
#endif // #if defined(__cplusplus)
-#endif // #ifndef liblldb_ReadWriteLock_h_
+#endif // #ifndef liblldb_ProcessRunLock_h_
Removed: lldb/trunk/include/lldb/Host/ReadWriteLock.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/ReadWriteLock.h?rev=187376&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Host/ReadWriteLock.h (original)
+++ lldb/trunk/include/lldb/Host/ReadWriteLock.h (removed)
@@ -1,146 +0,0 @@
-//===-- ReadWriteLock.h -----------------------------------------*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef liblldb_ReadWriteLock_h_
-#define liblldb_ReadWriteLock_h_
-#if defined(__cplusplus)
-
-#include "lldb/Host/Mutex.h"
-#include "lldb/Host/Condition.h"
-#include <pthread.h>
-#include <stdint.h>
-#include <time.h>
-
-//----------------------------------------------------------------------
-/// Enumerations for broadcasting.
-//----------------------------------------------------------------------
-namespace lldb_private {
-
-//----------------------------------------------------------------------
-/// @class ReadWriteLock ReadWriteLock.h "lldb/Host/ReadWriteLock.h"
-/// @brief A C++ wrapper class for providing threaded access to a value
-/// of type T.
-///
-/// A templatized class that provides multi-threaded access to a value
-/// of type T. Threads can efficiently wait for bits within T to be set
-/// or reset, or wait for T to be set to be equal/not equal to a
-/// specified values.
-//----------------------------------------------------------------------
-
-class ReadWriteLock
-{
-public:
- ReadWriteLock () :
- m_rwlock()
- {
- int err = ::pthread_rwlock_init(&m_rwlock, NULL); (void)err;
-//#if LLDB_CONFIGURATION_DEBUG
-// assert(err == 0);
-//#endif
- }
-
- ~ReadWriteLock ()
- {
- int err = ::pthread_rwlock_destroy (&m_rwlock); (void)err;
-//#if LLDB_CONFIGURATION_DEBUG
-// assert(err == 0);
-//#endif
- }
-
- bool
- ReadTryLock ()
- {
- return ::pthread_rwlock_tryrdlock (&m_rwlock) == 0;
- }
-
- bool
- ReadUnlock ()
- {
- return ::pthread_rwlock_unlock (&m_rwlock) == 0;
- }
-
- bool
- WriteLock()
- {
- return ::pthread_rwlock_wrlock (&m_rwlock) == 0;
- }
-
- bool
- WriteTryLock()
- {
- return ::pthread_rwlock_trywrlock (&m_rwlock) == 0;
- }
-
- bool
- WriteUnlock ()
- {
- return ::pthread_rwlock_unlock (&m_rwlock) == 0;
- }
-
- class ReadLocker
- {
- public:
- ReadLocker () :
- m_lock (NULL)
- {
- }
-
- ~ReadLocker()
- {
- Unlock();
- }
-
- // Try to lock the read lock, but only do so if there are no writers.
- bool
- TryLock (ReadWriteLock *lock)
- {
- if (m_lock)
- {
- if (m_lock == lock)
- return true; // We already have this lock locked
- else
- Unlock();
- }
- if (lock)
- {
- if (lock->ReadTryLock())
- {
- m_lock = lock;
- return true;
- }
- }
- return false;
- }
-
- protected:
- void
- Unlock ()
- {
- if (m_lock)
- {
- m_lock->ReadUnlock();
- m_lock = NULL;
- }
- }
-
- ReadWriteLock *m_lock;
- private:
- DISALLOW_COPY_AND_ASSIGN(ReadLocker);
- };
-
-protected:
- pthread_rwlock_t m_rwlock;
-private:
- DISALLOW_COPY_AND_ASSIGN(ReadWriteLock);
-};
-
-} // namespace lldb_private
-
-#endif // #if defined(__cplusplus)
-#endif // #ifndef liblldb_ReadWriteLock_h_
Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=187377&r1=187376&r2=187377&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Jul 29 15:58:06 2013
@@ -37,7 +37,7 @@
#include "lldb/Expression/IRDynamicChecks.h"
#include "lldb/Host/FileSpec.h"
#include "lldb/Host/Host.h"
-#include "lldb/Host/ReadWriteLock.h"
+#include "lldb/Host/ProcessRunLock.h"
#include "lldb/Interpreter/Args.h"
#include "lldb/Interpreter/Options.h"
#include "lldb/Target/ExecutionContextScope.h"
@@ -1392,7 +1392,7 @@ public:
// stopped can block waiting for the process to stop, or just
// try to lock it to see if they can immediately access the stopped
// process. If the try read lock fails, then the process is running.
- typedef ReadWriteLock::ReadLocker StopLocker;
+ typedef ProcessRunLock::ProcessRunLocker StopLocker;
// These two functions fill out the Broadcaster interface:
@@ -3516,7 +3516,7 @@ public:
void
ClearPreResumeActions ();
- ReadWriteLock &
+ ProcessRunLock &
GetRunLock ()
{
if (Host::GetCurrentThread() == m_private_state_thread)
@@ -3671,8 +3671,8 @@ protected:
LanguageRuntimeCollection m_language_runtimes;
std::unique_ptr<NextEventAction> m_next_event_action_ap;
std::vector<PreResumeCallbackAndBaton> m_pre_resume_actions;
- ReadWriteLock m_public_run_lock;
- ReadWriteLock m_private_run_lock;
+ ProcessRunLock m_public_run_lock;
+ ProcessRunLock m_private_run_lock;
Predicate<bool> m_currently_handling_event; // This predicate is set in HandlePrivateEvent while all its business is being done.
bool m_currently_handling_do_on_removals;
bool m_resume_requested; // If m_currently_handling_event or m_currently_handling_do_on_removals are true, Resume will only request a resume, using this flag to check.
Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=187377&r1=187376&r2=187377&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Mon Jul 29 15:58:06 2013
@@ -1157,10 +1157,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();
- m_private_run_lock.WriteTryLock(); // This will do nothing if already locked
- m_private_run_lock.WriteUnlock();
+ m_public_run_lock.TrySetRunning(); // This will do nothing if already locked
+ m_public_run_lock.SetStopped();
+ m_private_run_lock.TrySetRunning(); // This will do nothing if already locked
+ m_private_run_lock.SetStopped();
m_finalize_called = true;
}
@@ -1644,7 +1644,7 @@ Process::SetPublicState (StateType new_s
{
if (log)
log->Printf("Process::SetPublicState (%s) -- unlocking run lock for detach", StateAsCString(new_state));
- m_public_run_lock.WriteUnlock();
+ m_public_run_lock.SetStopped();
}
else
{
@@ -1656,7 +1656,7 @@ Process::SetPublicState (StateType new_s
{
if (log)
log->Printf("Process::SetPublicState (%s) -- unlocking run lock", StateAsCString(new_state));
- m_public_run_lock.WriteUnlock();
+ m_public_run_lock.SetStopped();
}
}
}
@@ -1669,11 +1669,11 @@ Process::Resume ()
Log *log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_STATE | LIBLLDB_LOG_PROCESS));
if (log)
log->Printf("Process::Resume -- locking run lock");
- if (!m_public_run_lock.WriteTryLock())
+ if (!m_public_run_lock.TrySetRunning())
{
Error error("Resume request failed - process still running.");
if (log)
- log->Printf ("Process::Resume: -- WriteTryLock failed, not resuming.");
+ log->Printf ("Process::Resume: -- TrySetRunning failed, not resuming.");
return error;
}
return PrivateResume();
@@ -1705,9 +1705,9 @@ Process::SetPrivateState (StateType new_
if (old_state_is_stopped != new_state_is_stopped)
{
if (new_state_is_stopped)
- m_private_run_lock.WriteUnlock();
+ m_private_run_lock.SetStopped();
else
- m_private_run_lock.WriteLock();
+ m_private_run_lock.SetRunning();
}
if (state_changed)
@@ -2867,7 +2867,7 @@ Process::Launch (const ProcessLaunchInfo
SetPublicState (eStateLaunching, restarted);
m_should_detach = false;
- if (m_public_run_lock.WriteTryLock())
+ if (m_public_run_lock.TrySetRunning())
{
// Now launch using these arguments.
error = DoLaunch (exe_module, launch_info);
@@ -3053,7 +3053,7 @@ Process::Attach (ProcessAttachInfo &atta
error = WillAttachToProcessWithName(process_name, wait_for_launch);
if (error.Success())
{
- if (m_public_run_lock.WriteTryLock())
+ if (m_public_run_lock.TrySetRunning())
{
m_should_detach = true;
const bool restarted = false;
@@ -3131,7 +3131,7 @@ Process::Attach (ProcessAttachInfo &atta
if (error.Success())
{
- if (m_public_run_lock.WriteTryLock())
+ if (m_public_run_lock.TrySetRunning())
{
// Now attach using these arguments.
m_should_detach = true;
@@ -3524,7 +3524,7 @@ Process::Detach (bool keep_stopped)
// the last events through the event system, in which case we might strand the write lock. Unlock
// it here so when we do to tear down the process we don't get an error destroying the lock.
- m_public_run_lock.WriteUnlock();
+ m_public_run_lock.SetStopped();
return error;
}
@@ -3582,7 +3582,7 @@ Process::Destroy ()
// If we have been interrupted (to kill us) in the middle of running, we may not end up propagating
// the last events through the event system, in which case we might strand the write lock. Unlock
// it here so when we do to tear down the process we don't get an error destroying the lock.
- m_public_run_lock.WriteUnlock();
+ m_public_run_lock.SetStopped();
}
m_destroy_in_process = false;
@@ -4059,7 +4059,7 @@ Process::RunPrivateStateThread ()
if (log)
log->Printf ("Process::%s (arg = %p, pid = %" PRIu64 ") thread exiting...", __FUNCTION__, this, GetID());
- m_public_run_lock.WriteUnlock();
+ m_public_run_lock.SetStopped();
m_private_state_control_wait.SetValue (true, eBroadcastAlways);
m_private_state_thread = LLDB_INVALID_HOST_THREAD;
return NULL;
More information about the lldb-commits
mailing list