[Lldb-commits] [lldb] r232023 - Fix ProcessIO test failures
Pavel Labath
labath at google.com
Thu Mar 12 03:12:42 PDT 2015
Author: labath
Date: Thu Mar 12 05:12:41 2015
New Revision: 232023
URL: http://llvm.org/viewvc/llvm-project?rev=232023&view=rev
Log:
Fix ProcessIO test failures
Summary:
There was a race condition regarding the output of the inferior process. The reading of the
output is performed on a separate thread, and there was no guarantee that the output will get
eventually consumed. Because of that, it was happening that calling Process::GetSTDOUT was not
returning anything even though the process was terminated and would definitely not produce any
further output. This was usually happening only under very heavy system load, but it can be
reproduced by placing an usleep in the stdio thread (Process::STDIOReadThreadBytesReceived).
This patch addresses this by adding synchronization capabilities to the Communication thread.
After calling Communication::SynchronizeWithReadThread one can be sure that all pending input has
been processed by the read thread. This function is then called after every public event which
stops the process to obtain the entire process output.
Test Plan: TestProcessIO.py should now succeed every time instead of flaking in and out.
Reviewers: clayborg, jingham
Subscribers: lldb-commits
Differential Revision: http://reviews.llvm.org/D8246
Modified:
lldb/trunk/include/lldb/Core/Communication.h
lldb/trunk/include/lldb/Core/Connection.h
lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h
lldb/trunk/source/Core/Communication.cpp
lldb/trunk/source/Target/Process.cpp
lldb/trunk/test/python_api/process/io/TestProcessIO.py
Modified: lldb/trunk/include/lldb/Core/Communication.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Communication.h?rev=232023&r1=232022&r2=232023&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Communication.h (original)
+++ lldb/trunk/include/lldb/Core/Communication.h Thu Mar 12 05:12:41 2015
@@ -91,6 +91,7 @@ public:
eBroadcastBitReadThreadDidExit = (1 << 2), ///< Sent by the read thread when it exits to inform clients.
eBroadcastBitReadThreadShouldExit = (1 << 3), ///< Sent by clients that need to cancel the read thread.
eBroadcastBitPacketAvailable = (1 << 4), ///< Sent when data received makes a complete packet.
+ eBroadcastBitNoMorePendingInput = (1 << 5), ///< Sent by the read thread to indicate all pending input has been processed.
kLoUserBroadcastBit = (1 << 16),///< Subclasses can used bits 31:16 for any needed events.
kHiUserBroadcastBit = (1 << 31),
eAllEventBits = 0xffffffff
@@ -321,6 +322,16 @@ public:
SetReadThreadBytesReceivedCallback (ReadThreadBytesReceived callback,
void *callback_baton);
+
+ //------------------------------------------------------------------
+ /// Wait for the read thread to process all outstanding data.
+ ///
+ /// After this function returns, the read thread has processed all data that
+ /// has been waiting in the Connection queue.
+ ///
+ //------------------------------------------------------------------
+ void SynchronizeWithReadThread ();
+
static const char *
ConnectionStatusAsCString (lldb::ConnectionStatus status);
@@ -354,9 +365,11 @@ protected:
lldb::ConnectionSP m_connection_sp; ///< The connection that is current in use by this communications class.
HostThread m_read_thread; ///< The read thread handle in case we need to cancel the thread.
std::atomic<bool> m_read_thread_enabled;
+ std::atomic<bool> m_read_thread_did_exit;
std::string m_bytes; ///< A buffer to cache bytes read in the ReadThread function.
Mutex m_bytes_mutex; ///< A mutex to protect multi-threaded access to the cached bytes.
Mutex m_write_mutex; ///< Don't let multiple threads write at the same time...
+ Mutex m_synchronize_mutex;
ReadThreadBytesReceived m_callback;
void *m_callback_baton;
bool m_close_on_eof;
Modified: lldb/trunk/include/lldb/Core/Connection.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Connection.h?rev=232023&r1=232022&r2=232023&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Connection.h (original)
+++ lldb/trunk/include/lldb/Core/Connection.h Thu Mar 12 05:12:41 2015
@@ -111,6 +111,13 @@ public:
/// The number of bytes to attempt to read, and also the max
/// number of bytes that can be placed into \a dst.
///
+ /// @param[in] timeout_usec
+ /// The number of microseconds to wait for the data.
+ ///
+ /// @param[out] status
+ /// On return, indicates whether the call was sucessful or terminated
+ /// due to some error condition.
+ ///
/// @param[out] error_ptr
/// A pointer to an error object that should be given an
/// approriate error value if this method returns zero. This
@@ -164,6 +171,22 @@ public:
virtual std::string
GetURI() = 0;
+ //------------------------------------------------------------------
+ /// Interrupts an ongoing Read() operation.
+ ///
+ /// If there is an ongoing read operation in another thread, this operation
+ /// return with status == eConnectionStatusInterrupted. Note that if there
+ /// data waiting to be read and an interrupt request is issued, the Read()
+ /// function will return the data immediately without processing the
+ /// interrupt request (which will remain queued for the next Read()
+ /// operation).
+ ///
+ /// @return
+ /// Returns true is the interrupt request was sucessful.
+ //------------------------------------------------------------------
+ virtual bool
+ InterruptRead() = 0;
+
private:
//------------------------------------------------------------------
// For Connection only
Modified: lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h?rev=232023&r1=232022&r2=232023&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h (original)
+++ lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h Thu Mar 12 05:12:41 2015
@@ -54,7 +54,7 @@ class ConnectionFileDescriptor : public
lldb::ConnectionStatus BytesAvailable(uint32_t timeout_usec, Error *error_ptr);
- bool InterruptRead();
+ bool InterruptRead() override;
lldb::IOObjectSP
GetReadObject()
Modified: lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h?rev=232023&r1=232022&r2=232023&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h (original)
+++ lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h Thu Mar 12 05:12:41 2015
@@ -40,7 +40,7 @@ class ConnectionGenericFile : public lld
virtual std::string GetURI();
- bool InterruptRead();
+ bool InterruptRead() override;
protected:
OVERLAPPED m_overlapped;
Modified: lldb/trunk/source/Core/Communication.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Communication.cpp?rev=232023&r1=232022&r2=232023&view=diff
==============================================================================
--- lldb/trunk/source/Core/Communication.cpp (original)
+++ lldb/trunk/source/Core/Communication.cpp Thu Mar 12 05:12:41 2015
@@ -39,9 +39,11 @@ Communication::Communication(const char
Broadcaster (NULL, name),
m_connection_sp (),
m_read_thread_enabled (false),
+ m_read_thread_did_exit (false),
m_bytes(),
m_bytes_mutex (Mutex::eMutexTypeRecursive),
m_write_mutex (Mutex::eMutexTypeNormal),
+ m_synchronize_mutex (Mutex::eMutexTypeNormal),
m_callback (NULL),
m_callback_baton (NULL),
m_close_on_eof (true)
@@ -56,6 +58,7 @@ Communication::Communication(const char
SetEventName (eBroadcastBitReadThreadDidExit, "read thread did exit");
SetEventName (eBroadcastBitReadThreadShouldExit, "read thread should exit");
SetEventName (eBroadcastBitPacketAvailable, "packet available");
+ SetEventName (eBroadcastBitNoMorePendingInput, "no more pending input");
CheckInWithManager();
}
@@ -244,6 +247,7 @@ Communication::StartReadThread (Error *e
snprintf(thread_name, sizeof(thread_name), "<lldb.comm.%s>", m_broadcaster_name.AsCString());
m_read_thread_enabled = true;
+ m_read_thread_did_exit = false;
m_read_thread = ThreadLauncher::LaunchThread(thread_name, Communication::ReadThread, this, error_ptr);
if (!m_read_thread.IsJoinable())
m_read_thread_enabled = false;
@@ -392,9 +396,13 @@ Communication::ReadThread (lldb::thread_
p,
Communication::ConnectionStatusAsCString (status));
break;
+ case eConnectionStatusInterrupted: // Synchronization signal from SynchronizeWithReadThread()
+ // The connection returns eConnectionStatusInterrupted only when there is no
+ // input pending to be read, so we can signal that.
+ comm->BroadcastEvent (eBroadcastBitNoMorePendingInput);
+ break;
case eConnectionStatusNoConnection: // No connection
case eConnectionStatusLostConnection: // Lost connection while connected to a valid connection
- case eConnectionStatusInterrupted: // Interrupted
done = true;
// Fall through...
case eConnectionStatusTimedOut: // Request timed out
@@ -410,7 +418,9 @@ Communication::ReadThread (lldb::thread_
if (log)
log->Printf ("%p Communication::ReadThread () thread exiting...", p);
+ comm->m_read_thread_did_exit = true;
// Let clients know that this thread is exiting
+ comm->BroadcastEvent (eBroadcastBitNoMorePendingInput);
comm->BroadcastEvent (eBroadcastBitReadThreadDidExit);
return NULL;
}
@@ -427,6 +437,28 @@ Communication::SetReadThreadBytesReceive
}
void
+Communication::SynchronizeWithReadThread ()
+{
+ // Only one thread can do the synchronization dance at a time.
+ Mutex::Locker locker(m_synchronize_mutex);
+
+ // First start listening for the synchronization event.
+ Listener listener("Communication::SyncronizeWithReadThread");
+ listener.StartListeningForEvents(this, eBroadcastBitNoMorePendingInput);
+
+ // If the thread is not running, there is no point in synchronizing.
+ if (!m_read_thread_enabled || m_read_thread_did_exit)
+ return;
+
+ // Notify the read thread.
+ m_connection_sp->InterruptRead();
+
+ // Wait for the synchronization event.
+ EventSP event_sp;
+ listener.WaitForEvent(NULL, event_sp);
+}
+
+void
Communication::SetConnection (Connection *connection)
{
Disconnect (NULL);
Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=232023&r1=232022&r2=232023&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Thu Mar 12 05:12:41 2015
@@ -4060,12 +4060,14 @@ Process::ShouldBroadcastEvent (Event *ev
switch (state)
{
- case eStateConnected:
- case eStateAttaching:
- case eStateLaunching:
case eStateDetached:
case eStateExited:
case eStateUnloaded:
+ m_stdio_communication.SynchronizeWithReadThread();
+ // fall-through
+ case eStateConnected:
+ case eStateAttaching:
+ case eStateLaunching:
// These events indicate changes in the state of the debugging session, always report them.
return_value = true;
break;
@@ -4125,6 +4127,7 @@ Process::ShouldBroadcastEvent (Event *ev
// 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.
+ m_stdio_communication.SynchronizeWithReadThread();
RefreshStateAfterStop ();
if (ProcessEventData::GetInterruptedFromEvent (event_ptr))
{
Modified: lldb/trunk/test/python_api/process/io/TestProcessIO.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/python_api/process/io/TestProcessIO.py?rev=232023&r1=232022&r2=232023&view=diff
==============================================================================
--- lldb/trunk/test/python_api/process/io/TestProcessIO.py (original)
+++ lldb/trunk/test/python_api/process/io/TestProcessIO.py Thu Mar 12 05:12:41 2015
@@ -21,7 +21,6 @@ class ProcessIOTestCase(TestBase):
@unittest2.skipIf(sys.platform.startswith("win32"), "stdio manipulation unsupported on Windows")
@python_api_test
@dwarf_test
- @expectedFailureLinux # this test fails 7/100 dosep runs
def test_stdin_by_api_with_dwarf(self):
"""Exercise SBProcess.PutSTDIN()."""
self.buildDwarf()
@@ -38,7 +37,6 @@ class ProcessIOTestCase(TestBase):
@unittest2.skipIf(sys.platform.startswith("win32"), "stdio manipulation unsupported on Windows")
@python_api_test
@dwarf_test
- @expectedFailureLinux # this test fails 4/100 dosep runs
def test_stdin_redirection_with_dwarf(self):
"""Exercise SBLaunchInfo::AddOpenFileAction() for STDIN without specifying STDOUT or STDERR."""
self.buildDwarf()
@@ -55,7 +53,6 @@ class ProcessIOTestCase(TestBase):
@unittest2.skipIf(sys.platform.startswith("win32"), "stdio manipulation unsupported on Windows")
@python_api_test
@dwarf_test
- @expectedFailureLinux # this test fails 2/100 dosep runs
def test_stdout_redirection_with_dwarf(self):
"""Exercise SBLaunchInfo::AddOpenFileAction() for STDOUT without specifying STDIN or STDERR."""
self.buildDwarf()
@@ -72,7 +69,6 @@ class ProcessIOTestCase(TestBase):
@unittest2.skipIf(sys.platform.startswith("win32"), "stdio manipulation unsupported on Windows")
@python_api_test
@dwarf_test
- @expectedFailureLinux # this test fails 5/100 dosep runs
def test_stderr_redirection_with_dwarf(self):
"""Exercise SBLaunchInfo::AddOpenFileAction() for STDERR without specifying STDIN or STDOUT."""
self.buildDwarf()
More information about the lldb-commits
mailing list