From a9e05eabf47817071b2bca5e106fec12891ca46f Mon Sep 17 00:00:00 2001
From: Ben Maurer <bmaurer@fb.com>
Date: Thu, 25 Jul 2013 15:47:58 -0700
Subject: [PATCH] Shutdown listen socket during segfault

During a segfault, we continue to accept incoming connections. This is
bad as the web server is unlikely to do a good job of serving these well.
Here we close the listen socket as soon as we discover the server is crashing.

A more robust way to do this might have been to have an array of FDs that
are listening for incoming connections. We could have then closed those
from the signal handling thread. This would complete the operation more
quickly and also handle a case where the libevent thread wasn't running.
However, this version was quicker to write.

I didn't understand why some signals didn't trigger the Segfaulting variable.
Does anybody have insight into that?
---
 hphp/runtime/base/crash_reporter.cpp    |  4 ++--
 hphp/runtime/base/crash_reporter.h      |  2 ++
 hphp/runtime/server/libevent_server.cpp | 20 ++++++++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/hphp/runtime/base/crash_reporter.cpp b/hphp/runtime/base/crash_reporter.cpp
index 1cc90f79a..fd7d0f8a7 100644
--- a/hphp/runtime/base/crash_reporter.cpp
+++ b/hphp/runtime/base/crash_reporter.cpp
@@ -28,16 +28,16 @@ namespace HPHP {
 
 //////////////////////////////////////////////////////////////////////
 
-bool SegFaulting = false;
+bool IsCrashing = false;
 
 static void bt_handler(int sig) {
   // In case we crash again in the signal hander or something
   signal(sig, SIG_DFL);
 
+  IsCrashing = true;
   // Generating a stack dumps significant time, try to stop threads
   // from flushing bad data or generating more faults meanwhile
   if (sig==SIGQUIT || sig==SIGILL || sig==SIGSEGV || sig==SIGBUS) {
-    SegFaulting = true;
     LightProcess::Close();
     // leave running for SIGTERM SIGFPE SIGABRT
   }
diff --git a/hphp/runtime/base/crash_reporter.h b/hphp/runtime/base/crash_reporter.h
index 11ebf4bab..8956b003c 100644
--- a/hphp/runtime/base/crash_reporter.h
+++ b/hphp/runtime/base/crash_reporter.h
@@ -21,6 +21,8 @@ namespace HPHP {
 
 //////////////////////////////////////////////////////////////////////
 
+extern bool IsCrashing;
+extern bool SegFaulting;
 void install_crash_reporter();
 
 //////////////////////////////////////////////////////////////////////
diff --git a/hphp/runtime/server/libevent_server.cpp b/hphp/runtime/server/libevent_server.cpp
index ba845dfac..2cb665987 100644
--- a/hphp/runtime/server/libevent_server.cpp
+++ b/hphp/runtime/server/libevent_server.cpp
@@ -18,6 +18,7 @@
 
 #include "hphp/runtime/base/runtime_option.h"
 #include "hphp/runtime/base/memory_manager.h"
+#include "hphp/runtime/base/crash_reporter.h"
 #include "hphp/runtime/server/server_stats.h"
 #include "hphp/runtime/server/http_protocol.h"
 #include "hphp/runtime/debugger/debugger.h"
@@ -377,6 +378,25 @@ int LibEventServer::getAcceptSocketSSL() {
 // request/response handling
 
 void LibEventServer::onRequest(struct evhttp_request *request) {
+  // If we are in the process of crashing, we want to reject incoming work.
+  // This will prompt the load balancers to choose another server. Using
+  // shutdown rather than close has the advantage that it makes fewer changes
+  // to the process (eg, it doesn't close the FD so if the FD number were
+  // corrupted it would be mostly harmless).
+  //
+  // Setting accept sock to -1 will leak FDs. But we're crashing anyways.
+  if (IsCrashing) {
+    if (m_accept_sock != -1) {
+      shutdown(m_accept_sock, SHUT_FBLISTEN);
+      m_accept_sock = -1;
+    }
+    if (m_accept_sock_ssl != -1) {
+      shutdown(m_accept_sock_ssl, SHUT_FBLISTEN);
+      m_accept_sock_ssl = -1;
+    }
+    return;
+  }
+
   if (RuntimeOption::EnableKeepAlive &&
       RuntimeOption::ConnectionTimeoutSeconds > 0) {
     // before processing request, set the connection timeout