From f3e19a5dfaa0b026dc3f01ad0439247b15723e27 Mon Sep 17 00:00:00 2001
From: Mike Magruder <mikemag@fb.com>
Date: Mon, 15 Jul 2013 12:25:33 -0700
Subject: [PATCH] Fix machine force attach
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I broke the operation of CmdMachine when we attach with the force flag with https://phabricator.fb.com/D851880. I consolidated some error handling logic around broken connections, etc., and in particular pulled throwing of an exception into forceQuit(). I failed to realize there was a single call site of forceQuit() that did not in fact throw an exception afterwards… the logic underneath of force attach. Switching the forceQuit() call there to stop() replaces the logic that was there previously, and restores the behavior.
---
 hphp/runtime/debugger/debugger.cpp       |  4 ++--
 hphp/runtime/debugger/debugger_proxy.cpp | 12 +++++++-----
 hphp/runtime/debugger/debugger_proxy.h   |  3 +--
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/hphp/runtime/debugger/debugger.cpp b/hphp/runtime/debugger/debugger.cpp
index eab811dd0..9781a6594 100644
--- a/hphp/runtime/debugger/debugger.cpp
+++ b/hphp/runtime/debugger/debugger.cpp
@@ -535,7 +535,7 @@ bool Debugger::switchSandbox(DebuggerProxyPtr proxy,
 bool Debugger::switchSandboxImpl(DebuggerProxyPtr proxy,
                                  const StringData* newSid,
                                  bool force) {
-  TRACE(2, "Debugger::switchSandboxImpln");
+  TRACE(2, "Debugger::switchSandboxImpl\n");
   // Take the new sandbox
   DebuggerProxyPtr otherProxy;
   {
@@ -565,7 +565,7 @@ bool Debugger::switchSandboxImpl(DebuggerProxyPtr proxy,
   setDebuggerFlag(newSid, true);
 
   if (otherProxy) {
-    otherProxy->forceQuit();
+    otherProxy->stop();
   }
 
   return true;
diff --git a/hphp/runtime/debugger/debugger_proxy.cpp b/hphp/runtime/debugger/debugger_proxy.cpp
index 5fbf6bd6e..081eb91ad 100644
--- a/hphp/runtime/debugger/debugger_proxy.cpp
+++ b/hphp/runtime/debugger/debugger_proxy.cpp
@@ -364,8 +364,10 @@ void DebuggerProxy::pollSignal() {
   TRACE_RB(2, "DebuggerProxy::pollSignal: ended\n");
 }
 
-// Ask this proxy to stop running and exit cleanly. Used during proxy cleanup,
-// and from the signal polling thread.
+// Ask this proxy to stop running and exit cleanly. Used during proxy
+// cleanup, sandbox switching, and from the signal polling
+// thread. This can be used from a thread which is not stopped at the
+// given proxy's interrupt.
 void DebuggerProxy::stop() {
   TRACE_RB(2, "DebuggerProxy::stop\n");
   DSandboxInfo invalid;
@@ -377,9 +379,9 @@ void DebuggerProxy::stop() {
 
 // Used to quit this proxy, typcially in response to either a quit command from
 // the client or loss of communication with the client. The proxy is removed
-// from the proxy map, ensuring no other threads can use the proxy. It stops
-// the proxy, and then tosses the client exit exception to ensure the current
-// request is terminated with a nice message.
+// from the proxy map, ensuring no other threads can use the proxy.
+// NB: It stops the proxy, and then tosses the client exit exception
+// to ensure the current request is terminated with a nice message.
 void DebuggerProxy::forceQuit() {
   TRACE_RB(2, "DebuggerProxy::forceQuit\n");
   Debugger::RemoveProxy(shared_from_this());
diff --git a/hphp/runtime/debugger/debugger_proxy.h b/hphp/runtime/debugger/debugger_proxy.h
index 86b1338bb..db6ee11d8 100644
--- a/hphp/runtime/debugger/debugger_proxy.h
+++ b/hphp/runtime/debugger/debugger_proxy.h
@@ -99,6 +99,7 @@ public:
 
   void checkStop();
   void forceQuit();
+  void stop();
 
   bool getClientConnectionInfo(VRefParam address, VRefParam port);
 
@@ -112,8 +113,6 @@ private:
 
   void changeBreakPointDepth(CmdInterrupt& cmd);
 
-  void stop();
-
   SmartPtr<Socket> getSocket() { return m_thrift.getSocket(); }
 
   bool m_stopped;