diff --git a/hphp/runtime/debugger/break_point.cpp b/hphp/runtime/debugger/break_point.cpp index 7befd2ca6..a0036e84a 100644 --- a/hphp/runtime/debugger/break_point.cpp +++ b/hphp/runtime/debugger/break_point.cpp @@ -353,7 +353,8 @@ bool BreakPointInfo::same(BreakPointInfoPtr bpi) { return desc() == bpi->desc(); } -bool BreakPointInfo::match(InterruptType interrupt, InterruptSite &site) { +bool BreakPointInfo::match(DebuggerProxy &proxy, InterruptType interrupt, + InterruptSite &site) { TRACE(2, "BreakPointInfo::match\n"); if (m_interruptType == interrupt) { switch (interrupt) { @@ -365,13 +366,13 @@ bool BreakPointInfo::match(InterruptType interrupt, InterruptSite &site) { case ExceptionThrown: return checkExceptionOrError(site.getError()) && - checkUrl(site.url()) && checkClause(); + checkUrl(site.url()) && checkClause(proxy); case BreakPointReached: { bool match = Match(site.getFile(), site.getFileLen(), m_file, m_regex, false) && checkLines(site.getLine0()) && checkStack(site) && - checkUrl(site.url()) && checkClause(); + checkUrl(site.url()) && checkClause(proxy); if (!getFuncName().empty()) { // function entry breakpoint @@ -999,7 +1000,7 @@ bool BreakPointInfo::checkStack(InterruptSite &site) { return true; } -bool BreakPointInfo::checkClause() { +bool BreakPointInfo::checkClause(DebuggerProxy &proxy) { TRACE(2, "BreakPointInfo::checkClause\n"); if (!m_clause.empty()) { if (m_php.empty()) { @@ -1014,7 +1015,8 @@ bool BreakPointInfo::checkClause() { // Don't hit more breakpoints while attempting to decide if we should stop // at this breakpoint. EvalBreakControl eval(true); - Variant ret = DebuggerProxy::ExecutePHP(m_php, output, false, 0); + Variant ret = proxy.ExecutePHP(m_php, output, 0, + DebuggerProxy::ExecutePHPFlagsNone); if (m_check) { return ret.toBoolean(); } diff --git a/hphp/runtime/debugger/break_point.h b/hphp/runtime/debugger/break_point.h index a22a24100..82b22ad09 100644 --- a/hphp/runtime/debugger/break_point.h +++ b/hphp/runtime/debugger/break_point.h @@ -119,6 +119,7 @@ private: DECLARE_BOOST_TYPES(DFunctionInfo); DECLARE_BOOST_TYPES(BreakPointInfo); +DECLARE_BOOST_TYPES(DebuggerProxy); class BreakPointInfo { public: // The state of the break point @@ -157,7 +158,8 @@ public: bool valid(); bool same(BreakPointInfoPtr bpi); - bool match(InterruptType interrupt, InterruptSite &site); + bool match(DebuggerProxy &proxy, InterruptType interrupt, + InterruptSite &site); int index() const { return m_index;} std::string state(bool padding) const; @@ -242,7 +244,7 @@ private: bool checkUrl(std::string &url); bool checkLines(int line); bool checkStack(InterruptSite &site); - bool checkClause(); + bool checkClause(DebuggerProxy &proxy); }; /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/debugger/cmd/cmd_eval.cpp b/hphp/runtime/debugger/cmd/cmd_eval.cpp index 737a3f967..192366348 100644 --- a/hphp/runtime/debugger/cmd/cmd_eval.cpp +++ b/hphp/runtime/debugger/cmd/cmd_eval.cpp @@ -67,11 +67,16 @@ void CmdEval::setClientOutput(DebuggerClient &client) { client.setOTValues(values.create()); } +// NB: unlike most other commands, the client expects that more interrupts +// can occur while we're doing the server-side work for an eval. bool CmdEval::onServer(DebuggerProxy &proxy) { PCFilter* locSave = g_vmContext->m_lastLocFilter; g_vmContext->m_lastLocFilter = new PCFilter(); g_vmContext->setDebuggerBypassCheck(m_bypassAccessCheck); - DebuggerProxy::ExecutePHP(m_body, m_output, !proxy.isLocal(), m_frame); + proxy.ExecutePHP(m_body, m_output, m_frame, + DebuggerProxy::ExecutePHPFlagsAtInterrupt | + (!proxy.isLocal() ? DebuggerProxy::ExecutePHPFlagsLog : + DebuggerProxy::ExecutePHPFlagsNone)); g_vmContext->setDebuggerBypassCheck(false); delete g_vmContext->m_lastLocFilter; g_vmContext->m_lastLocFilter = locSave; diff --git a/hphp/runtime/debugger/cmd/cmd_interrupt.cpp b/hphp/runtime/debugger/cmd/cmd_interrupt.cpp index 7be8b463b..51daaa04d 100644 --- a/hphp/runtime/debugger/cmd/cmd_interrupt.cpp +++ b/hphp/runtime/debugger/cmd/cmd_interrupt.cpp @@ -283,7 +283,8 @@ bool CmdInterrupt::onServer(DebuggerProxy &proxy) { return proxy.sendToClient(this); } -bool CmdInterrupt::shouldBreak(const BreakPointInfoPtrVec &bps, +bool CmdInterrupt::shouldBreak(DebuggerProxy &proxy, + const BreakPointInfoPtrVec &bps, int stackDepth) { switch (m_interrupt) { @@ -301,7 +302,7 @@ bool CmdInterrupt::shouldBreak(const BreakPointInfoPtrVec &bps, for (unsigned int i = 0; i < bps.size(); i++) { if (bps[i]->m_state != BreakPointInfo::Disabled && bps[i]->breakable(stackDepth) && - bps[i]->match(getInterruptType(), *getSite())) { + bps[i]->match(proxy, getInterruptType(), *getSite())) { BreakPointInfoPtr bp(new BreakPointInfo()); *bp = *bps[i]; // make a copy m_matched.push_back(bp); diff --git a/hphp/runtime/debugger/cmd/cmd_interrupt.h b/hphp/runtime/debugger/cmd/cmd_interrupt.h index 303e39539..edcbb9c67 100644 --- a/hphp/runtime/debugger/cmd/cmd_interrupt.h +++ b/hphp/runtime/debugger/cmd/cmd_interrupt.h @@ -47,7 +47,8 @@ public: virtual void setClientOutput(DebuggerClient &client); virtual bool onServer(DebuggerProxy &proxy); - bool shouldBreak(const BreakPointInfoPtrVec &bps, int stackDepth); + bool shouldBreak(DebuggerProxy &proxy, const BreakPointInfoPtrVec &bps, + int stackDepth); std::string getFileLine() const; InterruptSite *getSite() { return m_site;} diff --git a/hphp/runtime/debugger/cmd/cmd_print.cpp b/hphp/runtime/debugger/cmd/cmd_print.cpp index 19ded3e73..ebdfec5a2 100644 --- a/hphp/runtime/debugger/cmd/cmd_print.cpp +++ b/hphp/runtime/debugger/cmd/cmd_print.cpp @@ -373,14 +373,20 @@ void CmdPrint::setClientOutput(DebuggerClient &client) { client.setOTValues(values); } +// NB: unlike most other commands, the client expects that more interrupts +// can occur while we're doing the server-side work for a print. bool CmdPrint::onServer(DebuggerProxy &proxy) { PCFilter* locSave = g_vmContext->m_lastLocFilter; g_vmContext->m_lastLocFilter = new PCFilter(); g_vmContext->setDebuggerBypassCheck(m_bypassAccessCheck); { EvalBreakControl eval(m_noBreak); - m_ret = DebuggerProxy::ExecutePHP(DebuggerProxy::MakePHPReturn(m_body), - m_output, !proxy.isLocal(), m_frame); + m_ret = + proxy.ExecutePHP(DebuggerProxy::MakePHPReturn(m_body), + m_output, m_frame, + DebuggerProxy::ExecutePHPFlagsAtInterrupt | + (!proxy.isLocal() ? DebuggerProxy::ExecutePHPFlagsLog : + DebuggerProxy::ExecutePHPFlagsNone)); } g_vmContext->setDebuggerBypassCheck(false); delete g_vmContext->m_lastLocFilter; diff --git a/hphp/runtime/debugger/debugger_proxy.cpp b/hphp/runtime/debugger/debugger_proxy.cpp index 081eb91ad..25d01e52d 100644 --- a/hphp/runtime/debugger/debugger_proxy.cpp +++ b/hphp/runtime/debugger/debugger_proxy.cpp @@ -34,7 +34,7 @@ DebuggerProxy::DebuggerProxy(SmartPtr socket, bool local) : m_stopped(false), m_local(local), m_dummySandbox(nullptr), m_hasBreakPoints(false), m_threadMode(Normal), m_thread(0), m_signalThread(this, &DebuggerProxy::pollSignal), - m_signum(CmdSignal::SignalNone) { + m_okayToPoll(true), m_signum(CmdSignal::SignalNone) { TRACE(2, "DebuggerProxy::DebuggerProxy\n"); m_thrift.create(socket); m_dummyInfo = DSandboxInfo::CreateDummyInfo((int64_t)this); @@ -224,8 +224,12 @@ void DebuggerProxy::interrupt(CmdInterrupt &cmd) { if (checkFlowBreak(cmd)) { while (true) { try { - Lock lock(m_signalMutex); // Block the signal polling thread. - m_signum = CmdSignal::SignalNone; + // We're about to send the client an interrupt and start + // waiting for commands back from it. Disable signal polling + // during this time, since our protocol requires that only one + // thread talk to the client at a time. + disableSignalPolling(); + SCOPE_EXIT { enableSignalPolling(); }; processInterrupt(cmd); // If we've just processed a breakpoint, change it so we don't break at // it again until we're off this site. @@ -278,6 +282,24 @@ bool DebuggerProxy::sendToClient(DebuggerCommand *cmd) { return cmd->send(m_thrift); } +// Allow the signal polling thread to send CmdSignal messages to the +// client to see if it there is a signal to repond to. +void DebuggerProxy::enableSignalPolling() { + Lock lock(m_signalMutex); + m_okayToPoll = true; +} + +// Prevent the signal polling thread from sending CmdSignal messages +// to the client. Polling is disabled while a thread is stopped at an +// interrupt and responding to messages from the client. +void DebuggerProxy::disableSignalPolling() { + Lock lock(m_signalMutex); + m_okayToPoll = false; + // Drop any pending signal since we're about to start processing + // interrupts again anyway. + m_signum = CmdSignal::SignalNone; +} + void DebuggerProxy::startSignalThread() { TRACE(2, "DebuggerProxy::startSignalThread\n"); m_signalThread.start(); @@ -309,6 +331,9 @@ void DebuggerProxy::pollSignal() { // Block any threads that might be interrupting from communicating with the // client until we're done with this poll. Lock lock(m_signalMutex); + // Don't actually poll if another thread is already in a command + // processing loop with the client. + if (!m_okayToPoll) continue; // Send CmdSignal over to the client and wait for a response. CmdSignal cmd; @@ -409,6 +434,8 @@ std::string DebuggerProxy::MakePHPReturn(const std::string &php) { return "swapOutputBuffer(nullptr); g_context->setStdout(append_stdout, &sb); - if (log) { + if (flags & ExecutePHPFlagsLog) { Logger::SetThreadHook(append_stderr, &sb); } try { String code(php.c_str(), php.size(), CopyString); + // @TODO: enable this once task #2608250 is completed. +#if 0 + // We're about to start executing more PHP. This is typcially done + // in response to commands from the client, and the client expects + // those commands to send more interrupts since, of course, the + // user might want to debug the code we're about to run. If we're + // already processing an interrupt, enable signal polling around + // the execution fo the new PHP to ensure that we can handle + // signals while doing so. + if (flags & ExecutePHPFlagsAtInterrupt) enableSignalPolling(); + SCOPE_EXIT { + if (flags & ExecutePHPFlagsAtInterrupt) disableSignalPolling(); + }; +#endif g_vmContext->evalPHPDebugger((TypedValue*)&ret, code.get(), frame); } catch (InvalidFunctionCallException &e) { sb.append(Debugger::ColorStderr(String(e.what()))); @@ -670,7 +715,7 @@ Variant DebuggerProxy::ExecutePHP(const std::string &php, String &output, } g_context->setStdout(nullptr, nullptr); g_context->swapOutputBuffer(save); - if (log) { + if (flags & ExecutePHPFlagsLog) { Logger::SetThreadHook(nullptr, nullptr); } output = sb.detach(); @@ -684,7 +729,7 @@ BreakPointInfoPtr DebuggerProxy::getBreakPointAtCmd(CmdInterrupt& cmd) { for (unsigned int i = 0; i < m_breakpoints.size(); ++i) { BreakPointInfoPtr bp = m_breakpoints[i]; if (bp->m_state != BreakPointInfo::Disabled && - bp->match(cmd.getInterruptType(), *cmd.getSite())) { + bp->match(*this, cmd.getInterruptType(), *cmd.getSite())) { return bp; } } @@ -732,7 +777,7 @@ void DebuggerProxy::changeBreakPointDepth(CmdInterrupt& cmd) { // if the site changes, then update the breakpoint depth BreakPointInfoPtr bp = m_breakpoints[i]; if (bp->m_state != BreakPointInfo::Disabled && - !bp->match(cmd.getInterruptType(), *cmd.getSite())) { + !bp->match(*this, cmd.getInterruptType(), *cmd.getSite())) { m_breakpoints[i]->changeBreakPointDepth(getRealStackDepth()); } } diff --git a/hphp/runtime/debugger/debugger_proxy.h b/hphp/runtime/debugger/debugger_proxy.h index e73bf6733..0c026bbb1 100644 --- a/hphp/runtime/debugger/debugger_proxy.h +++ b/hphp/runtime/debugger/debugger_proxy.h @@ -58,8 +58,6 @@ public: static std::string MakePHP(const std::string &php); static std::string MakePHPReturn(const std::string &php); - static Variant ExecutePHP(const std::string &php, String &output, bool log, - int frame); public: DebuggerProxy(SmartPtr socket, bool local); @@ -103,11 +101,22 @@ public: bool getClientConnectionInfo(VRefParam address, VRefParam port); + enum ExecutePHPFlags { + ExecutePHPFlagsNone = 0x0, // No logging, not at an interrupt + ExecutePHPFlagsLog = 0x1, // Add logs to the output string + ExecutePHPFlagsAtInterrupt = 0x02 // Called when stopped at an interrupt + }; + + Variant ExecutePHP(const std::string &php, String &output, int frame, + int flags); + private: bool blockUntilOwn(CmdInterrupt &cmd, bool check); bool checkBreakPoints(CmdInterrupt &cmd); bool checkFlowBreak(CmdInterrupt &cmd); void processInterrupt(CmdInterrupt &cmd); + void enableSignalPolling(); + void disableSignalPolling(); DThreadInfoPtr createThreadInfo(const std::string &desc); @@ -137,6 +146,7 @@ private: Mutex m_signalMutex; // who can talk to client AsyncFunc m_signalThread; // polling signals from client + bool m_okayToPoll; // whether the polling thread can send polls to the client Mutex m_signumMutex; int m_signum;