From e0bf7767da36202747b6fc322f426bd2ca3fa041 Mon Sep 17 00:00:00 2001
From: Ihar Hrachyshka <ihrachys@redhat.com>
Date: Mon, 15 Jun 2015 18:53:54 +0200
Subject: [PATCH] daemon: avoid raising UnboundLocalError to callers

If something in the daemon_start() function fails before server variable
is initialized, we get the following exception:

UnboundLocalError: local variable 'server' referenced before assignment

We should not attempt to close connections or kill all threads for a
daemon that failed to start (or that hasn't even reached the moment of
the start).

Closes-Bug: #1465350
Change-Id: I7769e40c13e3bd740d5b8a949a61d1bcc127f137
---
 oslo_rootwrap/daemon.py              | 78 ++++++++++++++--------------
 oslo_rootwrap/tests/test_rootwrap.py | 17 ++++++
 2 files changed, 57 insertions(+), 38 deletions(-)

diff --git a/oslo_rootwrap/daemon.py b/oslo_rootwrap/daemon.py
index 21c3717..aa3612b 100644
--- a/oslo_rootwrap/daemon.py
+++ b/oslo_rootwrap/daemon.py
@@ -90,46 +90,48 @@ def daemon_start(config, filters):
         manager_cls = get_manager_class(config, filters)
         manager = manager_cls(address=socket_path)
         server = manager.get_server()
-        # allow everybody to connect to the socket
-        rw_rw_rw_ = (stat.S_IRUSR | stat.S_IWUSR |
-                     stat.S_IRGRP | stat.S_IWGRP |
-                     stat.S_IROTH | stat.S_IWOTH)
-        os.chmod(socket_path, rw_rw_rw_)
         try:
-            # In Python 3 we have to use buffer to push in bytes directly
-            stdout = sys.stdout.buffer
-        except AttributeError:
-            stdout = sys.stdout
-        stdout.write(socket_path.encode('utf-8'))
-        stdout.write(b'\n')
-        stdout.write(bytes(server.authkey))
-        sys.stdin.close()
-        sys.stdout.close()
-        sys.stderr.close()
-        # Gracefully shutdown on INT or TERM signals
-        stop = functools.partial(daemon_stop, server)
-        signal.signal(signal.SIGTERM, stop)
-        signal.signal(signal.SIGINT, stop)
-        LOG.info("Starting rootwrap daemon main loop")
-        server.serve_forever()
-    finally:
-        conn = server.listener
-        # This will break accept() loop with EOFError if it was not in the main
-        # thread (as in Python 3.x)
-        conn.close()
-        # Closing all currently connected client sockets for reading to break
-        # worker threads blocked on recv()
-        for cl_conn in conn.get_accepted():
+            # allow everybody to connect to the socket
+            rw_rw_rw_ = (stat.S_IRUSR | stat.S_IWUSR |
+                         stat.S_IRGRP | stat.S_IWGRP |
+                         stat.S_IROTH | stat.S_IWOTH)
+            os.chmod(socket_path, rw_rw_rw_)
             try:
-                cl_conn.half_close()
-            except Exception:
-                # Most likely the socket have already been closed
-                LOG.debug("Failed to close connection")
-        LOG.info("Waiting for all client threads to finish.")
-        for thread in threading.enumerate():
-            if thread.daemon:
-                LOG.debug("Joining thread %s", thread)
-                thread.join()
+                # In Python 3 we have to use buffer to push in bytes directly
+                stdout = sys.stdout.buffer
+            except AttributeError:
+                stdout = sys.stdout
+            stdout.write(socket_path.encode('utf-8'))
+            stdout.write(b'\n')
+            stdout.write(bytes(server.authkey))
+            sys.stdin.close()
+            sys.stdout.close()
+            sys.stderr.close()
+            # Gracefully shutdown on INT or TERM signals
+            stop = functools.partial(daemon_stop, server)
+            signal.signal(signal.SIGTERM, stop)
+            signal.signal(signal.SIGINT, stop)
+            LOG.info("Starting rootwrap daemon main loop")
+            server.serve_forever()
+        finally:
+            conn = server.listener
+            # This will break accept() loop with EOFError if it was not in the
+            # main thread (as in Python 3.x)
+            conn.close()
+            # Closing all currently connected client sockets for reading to
+            # break worker threads blocked on recv()
+            for cl_conn in conn.get_accepted():
+                try:
+                    cl_conn.half_close()
+                except Exception:
+                    # Most likely the socket have already been closed
+                    LOG.debug("Failed to close connection")
+            LOG.info("Waiting for all client threads to finish.")
+            for thread in threading.enumerate():
+                if thread.daemon:
+                    LOG.debug("Joining thread %s", thread)
+                    thread.join()
+    finally:
         LOG.debug("Removing temporary directory %s", temp_dir)
         shutil.rmtree(temp_dir)
 
diff --git a/oslo_rootwrap/tests/test_rootwrap.py b/oslo_rootwrap/tests/test_rootwrap.py
index 2dee792..d951545 100644
--- a/oslo_rootwrap/tests/test_rootwrap.py
+++ b/oslo_rootwrap/tests/test_rootwrap.py
@@ -24,6 +24,7 @@ from six import moves
 import testtools
 
 from oslo_rootwrap import cmd
+from oslo_rootwrap import daemon
 from oslo_rootwrap import filters
 from oslo_rootwrap import wrapper
 
@@ -583,3 +584,19 @@ class RunOneCommandTestCase(testtools.TestCase):
 
     def test_negative_returncode(self):
         self._test_returncode_helper(-1, 129)
+
+
+class DaemonCleanupException(Exception):
+    pass
+
+
+class DaemonCleanupTestCase(testtools.TestCase):
+
+    @mock.patch('os.chmod')
+    @mock.patch('shutil.rmtree')
+    @mock.patch('tempfile.mkdtemp')
+    @mock.patch('multiprocessing.managers.BaseManager.get_server',
+                side_effect=DaemonCleanupException)
+    def test_daemon_no_cleanup_for_uninitialized_server(self, gs, *args):
+        self.assertRaises(DaemonCleanupException, daemon.daemon_start,
+                          config=None, filters=None)