[Lldb-commits] [lldb] r242782 - [LLGS] Get rid of the stdio forwarding thread
Pavel Labath
labath at google.com
Tue Jul 21 06:20:25 PDT 2015
Author: labath
Date: Tue Jul 21 08:20:25 2015
New Revision: 242782
URL: http://llvm.org/viewvc/llvm-project?rev=242782&view=rev
Log:
[LLGS] Get rid of the stdio forwarding thread
Summary:
This commit removes the stdio forwarding thread in lldb-server in favor of a MainLoop callback.
As in some situations we need to forcibly flush the stream ( => Read() is called from multiple
places) and we still have multiple threads, I have had to additionally protect the communication
instance with a mutex.
Reviewers: ovyalov, tberghammer
Subscribers: lldb-commits
Differential Revision: http://reviews.llvm.org/D11296
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=242782&r1=242781&r2=242782&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp Tue Jul 21 08:20:25 2015
@@ -731,10 +731,7 @@ GDBRemoteCommunicationServerLLGS::Handle
if (log)
log->Printf ("GDBRemoteCommunicationServerLLGS::%s called", __FUNCTION__);
- // Send the exit result, and don't flush output.
- // Note: flushing output here would join the inferior stdio reflection thread, which
- // would gunk up the waitpid monitor thread that is calling this.
- PacketResult result = SendStopReasonForState (StateType::eStateExited, false);
+ PacketResult result = SendStopReasonForState(StateType::eStateExited);
if (result != PacketResult::Success)
{
if (log)
@@ -752,22 +749,8 @@ GDBRemoteCommunicationServerLLGS::Handle
}
}
- // FIXME can't do this yet - since process state propagation is currently
- // synchronous, it is running off the NativeProcessProtocol's innards and
- // will tear down the NPP while it still has code to execute.
-#if 0
- // Clear the NativeProcessProtocol pointer.
- {
- Mutex::Locker locker (m_debugged_process_mutex);
- m_debugged_process_sp.reset();
- }
-#endif
-
// Close the pipe to the inferior terminal i/o if we launched it
- // and set one up. Otherwise, 'k' and its flush of stdio could
- // end up waiting on a thread join that will never end. Consider
- // adding a timeout to the connection thread join call so we
- // can avoid that scenario altogether.
+ // and set one up.
MaybeCloseInferiorTerminalConnection ();
// We are ready to exit the debug monitor.
@@ -793,7 +776,7 @@ GDBRemoteCommunicationServerLLGS::Handle
break;
default:
// In all other cases, send the stop reason.
- PacketResult result = SendStopReasonForState (StateType::eStateStopped, false);
+ PacketResult result = SendStopReasonForState(StateType::eStateStopped);
if (result != PacketResult::Success)
{
if (log)
@@ -819,7 +802,7 @@ GDBRemoteCommunicationServerLLGS::Proces
// Make sure we get all of the pending stdout/stderr from the inferior
// and send it to the lldb host before we send the state change
// notification
- m_stdio_communication.SynchronizeWithReadThread();
+ SendProcessOutput();
switch (state)
{
@@ -864,7 +847,6 @@ GDBRemoteCommunicationServerLLGS::DataAv
if(log)
log->Printf("GDBRemoteCommunicationServerLLGS::%s handshake with client failed, exiting",
__FUNCTION__);
- m_read_handle_up.reset();
m_mainloop.RequestTermination();
return;
}
@@ -885,7 +867,6 @@ GDBRemoteCommunicationServerLLGS::DataAv
if(log)
log->Printf("GDBRemoteCommunicationServerLLGS::%s processing a packet failed: %s",
__FUNCTION__, error.AsCString());
- m_read_handle_up.reset();
m_mainloop.RequestTermination();
break;
}
@@ -899,7 +880,7 @@ GDBRemoteCommunicationServerLLGS::Initia
GDBRemoteCommunicationServer::SetConnection(connection.release());
Error error;
- m_read_handle_up = m_mainloop.RegisterReadObject(read_object_sp,
+ m_network_handle_up = m_mainloop.RegisterReadObject(read_object_sp,
[this] (MainLoopBase &) { DataAvailableCallback(); }, error);
return error;
}
@@ -925,7 +906,7 @@ GDBRemoteCommunicationServerLLGS::SetSTD
{
Error error;
- // Set up the Read Thread for reading/handling process I/O
+ // Set up the reading/handling of process I/O
std::unique_ptr<ConnectionFileDescriptor> conn_up (new ConnectionFileDescriptor (fd, true));
if (!conn_up)
{
@@ -933,6 +914,7 @@ GDBRemoteCommunicationServerLLGS::SetSTD
return error;
}
+ Mutex::Locker locker(m_stdio_communication_mutex);
m_stdio_communication.SetCloseOnEOF (false);
m_stdio_communication.SetConnection (conn_up.release());
if (!m_stdio_communication.IsConnected ())
@@ -951,19 +933,55 @@ GDBRemoteCommunicationServerLLGS::SetSTD
)
{
// output from the process must be forwarded over gdb-remote
- // create a thread to read the handle and send the data
- m_stdio_communication.SetReadThreadBytesReceivedCallback (STDIOReadThreadBytesReceived, this);
- m_stdio_communication.StartReadThread();
+ m_stdio_handle_up = m_mainloop.RegisterReadObject(
+ m_stdio_communication.GetConnection()->GetReadObject(),
+ [this] (MainLoopBase &) { SendProcessOutput(); }, error);
+ if (! m_stdio_handle_up)
+ {
+ m_stdio_communication.Disconnect();
+ return error;
+ }
}
- return error;
+ return Error();
}
void
-GDBRemoteCommunicationServerLLGS::STDIOReadThreadBytesReceived (void *baton, const void *src, size_t src_len)
+GDBRemoteCommunicationServerLLGS::StopSTDIOForwarding()
{
- GDBRemoteCommunicationServerLLGS *server = reinterpret_cast<GDBRemoteCommunicationServerLLGS*> (baton);
- static_cast<void> (server->SendONotification (static_cast<const char *>(src), src_len));
+ Mutex::Locker locker(m_stdio_communication_mutex);
+ m_stdio_handle_up.reset();
+}
+
+void
+GDBRemoteCommunicationServerLLGS::SendProcessOutput()
+{
+ char buffer[1024];
+ ConnectionStatus status;
+ Error error;
+ Mutex::Locker locker(m_stdio_communication_mutex);
+ while (true)
+ {
+ size_t bytes_read = m_stdio_communication.Read(buffer, sizeof buffer, 0, status, &error);
+ switch (status)
+ {
+ case eConnectionStatusSuccess:
+ SendONotification(buffer, bytes_read);
+ break;
+ case eConnectionStatusLostConnection:
+ case eConnectionStatusEndOfFile:
+ case eConnectionStatusError:
+ case eConnectionStatusNoConnection:
+ if (Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS))
+ log->Printf("GDBRemoteCommunicationServerLLGS::%s Stopping stdio forwarding as communication returned status %d (error: %s)", __FUNCTION__, status, error.AsCString());
+ m_stdio_handle_up.reset();
+ return;
+
+ case eConnectionStatusInterrupted:
+ case eConnectionStatusTimedOut:
+ return;
+ }
+ }
}
GDBRemoteCommunication::PacketResult
@@ -1051,7 +1069,7 @@ GDBRemoteCommunicationServerLLGS::Handle
}
}
- FlushInferiorOutput ();
+ StopSTDIOForwarding();
// No OK response for kill packet.
// return SendOKResponse ();
@@ -1384,11 +1402,11 @@ GDBRemoteCommunicationServerLLGS::Handle
if (!m_debugged_process_sp)
return SendErrorResponse (02);
- return SendStopReasonForState (m_debugged_process_sp->GetState (), true);
+ return SendStopReasonForState (m_debugged_process_sp->GetState());
}
GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::SendStopReasonForState (lldb::StateType process_state, bool flush_on_exit)
+GDBRemoteCommunicationServerLLGS::SendStopReasonForState (lldb::StateType process_state)
{
Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
@@ -1417,8 +1435,6 @@ GDBRemoteCommunicationServerLLGS::SendSt
case eStateInvalid:
case eStateUnloaded:
case eStateExited:
- if (flush_on_exit)
- FlushInferiorOutput ();
return SendWResponse(m_debugged_process_sp.get());
default:
@@ -1879,6 +1895,7 @@ GDBRemoteCommunicationServerLLGS::Handle
// TODO: enqueue this block in circular buffer and send window size to remote host
ConnectionStatus status;
Error error;
+ Mutex::Locker locker(m_stdio_communication_mutex);
m_stdio_communication.Write(tmp, read, status, &error);
if (error.Fail())
{
@@ -2623,7 +2640,7 @@ GDBRemoteCommunicationServerLLGS::Handle
}
// Notify we attached by sending a stop packet.
- return SendStopReasonForState (m_debugged_process_sp->GetState (), true);
+ return SendStopReasonForState (m_debugged_process_sp->GetState ());
}
GDBRemoteCommunication::PacketResult
@@ -2631,6 +2648,8 @@ GDBRemoteCommunicationServerLLGS::Handle
{
Log *log (GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PROCESS));
+ StopSTDIOForwarding();
+
// Scope for mutex locker.
Mutex::Locker locker (m_spawned_pids_mutex);
@@ -2671,11 +2690,6 @@ GDBRemoteCommunicationServerLLGS::Handle
return SendIllFormedResponse (packet, "Invalid pid");
}
- if (m_stdio_communication.IsConnected ())
- {
- m_stdio_communication.StopReadThread ();
- }
-
const Error error = m_debugged_process_sp->Detach ();
if (error.Fail ())
{
@@ -2840,25 +2854,11 @@ GDBRemoteCommunicationServerLLGS::Handle
}
void
-GDBRemoteCommunicationServerLLGS::FlushInferiorOutput ()
-{
- // If we're not monitoring an inferior's terminal, ignore this.
- if (!m_stdio_communication.IsConnected())
- return;
-
- Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
- if (log)
- log->Printf ("GDBRemoteCommunicationServerLLGS::%s() called", __FUNCTION__);
-
- // FIXME implement a timeout on the join.
- m_stdio_communication.JoinReadThread();
-}
-
-void
GDBRemoteCommunicationServerLLGS::MaybeCloseInferiorTerminalConnection ()
{
Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+ Mutex::Locker locker(m_stdio_communication_mutex);
// Tell the stdio connection to shut down.
if (m_stdio_communication.IsConnected())
{
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h?rev=242782&r1=242781&r2=242782&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h Tue Jul 21 08:20:25 2015
@@ -119,12 +119,16 @@ public:
protected:
lldb::PlatformSP m_platform_sp;
MainLoop &m_mainloop;
- MainLoop::ReadHandleUP m_read_handle_up;
+ MainLoop::ReadHandleUP m_network_handle_up;
lldb::tid_t m_current_tid;
lldb::tid_t m_continue_tid;
Mutex m_debugged_process_mutex;
NativeProcessProtocolSP m_debugged_process_sp;
+
+ Mutex m_stdio_communication_mutex; // Protects m_stdio_communication and m_stdio_handle_up
Communication m_stdio_communication;
+ MainLoop::ReadHandleUP m_stdio_handle_up;
+
lldb::StateType m_inferior_prev_state;
lldb::DataBufferSP m_active_auxv_buffer_sp;
Mutex m_saved_registers_mutex;
@@ -142,7 +146,7 @@ protected:
SendStopReplyPacketForThread (lldb::tid_t tid);
PacketResult
- SendStopReasonForState (lldb::StateType process_state, bool flush_on_exit);
+ SendStopReasonForState (lldb::StateType process_state);
PacketResult
Handle_k (StringExtractorGDBRemote &packet);
@@ -264,9 +268,6 @@ protected:
Error
SetSTDIOFileDescriptor (int fd);
- static void
- STDIOReadThreadBytesReceived (void *baton, const void *src, size_t src_len);
-
FileSpec
FindModuleFile (const std::string& module_path, const ArchSpec& arch) override;
@@ -287,9 +288,6 @@ private:
void
HandleInferiorState_Stopped (NativeProcessProtocol *process);
- void
- FlushInferiorOutput ();
-
NativeThreadProtocolSP
GetThreadFromSuffix (StringExtractorGDBRemote &packet);
@@ -308,6 +306,12 @@ private:
void
DataAvailableCallback ();
+ void
+ SendProcessOutput ();
+
+ void
+ StopSTDIOForwarding();
+
//------------------------------------------------------------------
// For GDBRemoteCommunicationServerLLGS only
//------------------------------------------------------------------
More information about the lldb-commits
mailing list