From 64da297cedf0d3e672bf982851c6edef55cf94d3 Mon Sep 17 00:00:00 2001 From: Mike Magruder Date: Wed, 15 May 2013 10:40:54 -0700 Subject: [PATCH] Adjust some debugger logging around the communications channel, and ensure we check all error conditions before attempting to read data from the socket The error handling around the communication channel between client and server is a bit sketchy right now. We have some cases that are reasonable when the client is quitting and the socket is closing, and some cases where it would be surprising for this to happen. This leads to spurious messages printed out when quitting the client normally. For now, switch some of these to trace messages instead of log messages. Task 2398988 tracks cleaning this up properly. --- hphp/runtime/eval/debugger/debugger_command.cpp | 3 ++- hphp/runtime/eval/debugger/debugger_proxy.cpp | 16 ++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/hphp/runtime/eval/debugger/debugger_command.cpp b/hphp/runtime/eval/debugger/debugger_command.cpp index 9a7f40b14..ef20e7a18 100644 --- a/hphp/runtime/eval/debugger/debugger_command.cpp +++ b/hphp/runtime/eval/debugger/debugger_command.cpp @@ -76,7 +76,8 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift, if (ret == 0) { return false; } - if (ret == -1 || !(fds[0].revents & POLLIN)) { + // Any error bits set indicate that we have nothing to read, so bail early. + if ((ret == -1) || (fds[0].revents != POLLIN)) { return errno != EINTR; // treat signals as timeouts } diff --git a/hphp/runtime/eval/debugger/debugger_proxy.cpp b/hphp/runtime/eval/debugger/debugger_proxy.cpp index cac4248f2..67ce8d4e6 100644 --- a/hphp/runtime/eval/debugger/debugger_proxy.cpp +++ b/hphp/runtime/eval/debugger/debugger_proxy.cpp @@ -349,7 +349,7 @@ void DebuggerProxy::pollSignal() { // Send CmdSignal over to the client and wait for a response. CmdSignal cmd; if (!cmd.onServer(this)) { - Logger::Error("Failed to send CmdSignal to client, socket error"); + TRACE(1, "Failed to send CmdSignal to client, socket error"); break; } @@ -359,21 +359,21 @@ void DebuggerProxy::pollSignal() { while (!DebuggerCommand::Receive(m_thrift, res, "DebuggerProxy::pollSignal()")) { if (m_stopped) { - Logger::Warning("DebuggerProxy signal thread asked to stop while " - "waiting for CmdSignal back from the client"); + TRACE(1, "DebuggerProxy signal thread asked to stop while " + "waiting for CmdSignal back from the client"); break; } } if (!res) { if (!m_stopped) { - Logger::Error("Failed to get CmdSignal back from client, socket error"); + TRACE(1, "Failed to get CmdSignal back from client, socket error"); } break; } CmdSignalPtr sig = dynamic_pointer_cast(res); if (!sig) { - Logger::Error("bad response from signal polling: %d", res->getType()); + TRACE(1, "bad response from signal polling: %d", res->getType()); break; } @@ -385,8 +385,8 @@ void DebuggerProxy::pollSignal() { } } if (!m_stopped) { - Logger::Error("DebuggerProxy signal thread has lost communication with the " - "client, stopping proxy."); + TRACE(1, "DebuggerProxy signal thread has lost communication with the " + "client, stopping proxy."); forceQuit(); } } @@ -621,7 +621,7 @@ void DebuggerProxy::processInterrupt(CmdInterrupt &cmd) { cmdFailure = true; } } else { - Logger::Error("Failed to receive cmd from client, socket error"); + TRACE(1, "Failed to receive cmd from client, socket error"); cmdFailure = true; } } catch (const DebuggerException &e) {