From 114041b18e878a7057af6a0d4bf8d0e84c001fad Mon Sep 17 00:00:00 2001
From: Alexis Lee <lxsli@hpe.com>
Date: Thu, 2 Jun 2016 17:09:23 +0100
Subject: [PATCH] Stay alive on double SIGHUP

There's a short window where sending a second SIGHUP will cause the
process to exit rather than reload config. This causes some confusion
to operators/testers and is hazardous for use with a config management
system. It would be easy to write plays that accidentally, even
intermittently, cause the service to exit.

Disallow this for SIGHUP. Graceful/fast shutdowns can still be
achieved with SIGTERM/SIGINT.

Change-Id: Ia2146b25b857217d498bf785c17bd0b629174c86
---
 oslo_service/service.py            | 22 ++++++++++++++++----
 oslo_service/tests/test_service.py | 32 ++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/oslo_service/service.py b/oslo_service/service.py
index 8f1dc27e..d175c7bf 100644
--- a/oslo_service/service.py
+++ b/oslo_service/service.py
@@ -289,6 +289,7 @@ class ServiceLauncher(Launcher):
 
     def handle_signal(self):
         """Set self._handle_signal as a signal handler."""
+        self.signal_handler.clear()
         self.signal_handler.add_handler('SIGTERM', self._graceful_shutdown)
         self.signal_handler.add_handler('SIGINT', self._fast_exit)
         self.signal_handler.add_handler('SIGHUP', self._reload_service)
@@ -372,13 +373,13 @@ class ProcessLauncher(object):
 
     def handle_signal(self):
         """Add instance's signal handlers to class handlers."""
-        self.signal_handler.add_handlers(('SIGTERM', 'SIGHUP'),
-                                         self._handle_signal)
+        self.signal_handler.add_handler('SIGTERM', self._handle_term)
+        self.signal_handler.add_handler('SIGHUP', self._handle_hup)
         self.signal_handler.add_handler('SIGINT', self._fast_exit)
         self.signal_handler.add_handler('SIGALRM', self._on_alarm_exit)
 
-    def _handle_signal(self, signo, frame):
-        """Set signal handlers.
+    def _handle_term(self, signo, frame):
+        """Handle a TERM event.
 
         :param signo: signal number
         :param frame: current stack frame
@@ -389,6 +390,19 @@ class ProcessLauncher(object):
         # Allow the process to be killed again and die from natural causes
         self.signal_handler.clear()
 
+    def _handle_hup(self, signo, frame):
+        """Handle a HUP event.
+
+        :param signo: signal number
+        :param frame: current stack frame
+        """
+        self.sigcaught = signo
+        self.running = False
+
+        # Do NOT clear the signal_handler, allowing multiple SIGHUPs to be
+        # received swiftly. If a non-HUP is received before #wait loops, the
+        # second event will "overwrite" the HUP. This is fine.
+
     def _fast_exit(self, signo, frame):
         LOG.info(_LI('Caught SIGINT signal, instantaneous exiting'))
         os._exit(1)
diff --git a/oslo_service/tests/test_service.py b/oslo_service/tests/test_service.py
index d955d7c9..a0a139ef 100644
--- a/oslo_service/tests/test_service.py
+++ b/oslo_service/tests/test_service.py
@@ -535,6 +535,38 @@ class ProcessLauncherTest(base.ServiceBaseTestCase):
         serv = FooService()
         self.assertRaises(TypeError, launcher.launch_service, serv, 0)
 
+    @mock.patch("oslo_service.service.ProcessLauncher._start_child")
+    @mock.patch("oslo_service.service.ProcessLauncher.handle_signal")
+    @mock.patch("eventlet.greenio.GreenPipe")
+    @mock.patch("os.pipe")
+    def test_double_sighup(self, pipe_mock, green_pipe_mock,
+                           handle_signal_mock, start_child_mock):
+        # Test that issuing two SIGHUPs in a row does not exit; then send a
+        # TERM that does cause an exit.
+        pipe_mock.return_value = [None, None]
+        launcher = service.ProcessLauncher(self.conf)
+        serv = _Service()
+        launcher.launch_service(serv, workers=0)
+
+        def stager():
+            # -1: start state
+            # 0: post-init
+            # 1: first HUP sent
+            # 2: second HUP sent
+            # 3: TERM sent
+            stager.stage += 1
+            if stager.stage < 3:
+                launcher._handle_hup(1, mock.sentinel.frame)
+            elif stager.stage == 3:
+                launcher._handle_term(15, mock.sentinel.frame)
+            else:
+                self.fail("TERM did not kill launcher")
+        stager.stage = -1
+        handle_signal_mock.side_effect = stager
+
+        launcher.wait()
+        self.assertEqual(3, stager.stage)
+
 
 class GracefulShutdownTestService(service.Service):
     def __init__(self):