From 301a96f664d58b4ccad8e3cbf5d5a889cc76790f Mon Sep 17 00:00:00 2001 From: "Jay S. Bryant" Date: Tue, 30 Sep 2014 15:08:59 -0500 Subject: [PATCH] Ensure sys.exit called in fork_child after exception Currently, the fork_child() function in auditor.py does not handle the case where run_audit() encounters an exception properly. A simple case is where the /srv directory is set with permissions such that the 'swift' user cannot access it. Such a situation causes a os.listdir() to return an OSError exception. When this happens the fork_child() process does not run to completion and sys.exit() is not executed. The process that was forked off continues to run as a result. Execution goes back up to the audit_loop function which restarts the whole process. The end result is an increasing number of processes on the system until the parent is terminated. This can quickly exhaust the process descriptors on a system. This change wraps run_audit() in a try block and adds an exception handler that prints what exception was encountered. The sys.exit() was moved to a finally: block so that it will always be run, avoiding the creation of zombies. Change-Id: I89d7cd27112445893852e62df857c3d5262c27b3 Closes-bug: 1375348 --- swift/obj/auditor.py | 8 ++++++-- test/unit/obj/test_auditor.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/swift/obj/auditor.py b/swift/obj/auditor.py index fce1d85b59..4c78d07af3 100644 --- a/swift/obj/auditor.py +++ b/swift/obj/auditor.py @@ -257,8 +257,12 @@ class ObjectAuditor(Daemon): signal.signal(signal.SIGTERM, signal.SIG_DFL) if zero_byte_fps: kwargs['zero_byte_fps'] = self.conf_zero_byte_fps - self.run_audit(**kwargs) - sys.exit() + try: + self.run_audit(**kwargs) + except Exception as e: + self.logger.error(_("ERROR: Unable to run auditing: %s") % e) + finally: + sys.exit() def audit_loop(self, parent, zbo_fps, override_devices=None, **kwargs): """Parallel audit loop""" diff --git a/test/unit/obj/test_auditor.py b/test/unit/obj/test_auditor.py index e65beab84e..e8f8a2b16a 100644 --- a/test/unit/obj/test_auditor.py +++ b/test/unit/obj/test_auditor.py @@ -487,6 +487,18 @@ class TestAuditor(unittest.TestCase): finally: auditor.diskfile.DiskFile = was_df + @mock.patch.object(auditor.ObjectAuditor, 'run_audit') + @mock.patch('os.fork', return_value=0) + def test_with_inaccessible_object_location(self, mock_os_fork, + mock_run_audit): + # Need to ensure that any failures in run_audit do + # not prevent sys.exit() from running. Otherwise we get + # zombie processes. + e = OSError('permission denied') + mock_run_audit.side_effect = e + self.auditor = auditor.ObjectAuditor(self.conf) + self.assertRaises(SystemExit, self.auditor.fork_child, self) + def test_with_tombstone(self): ts_file_path = self.setup_bad_zero_byte(with_ts=True) self.assertTrue(ts_file_path.endswith('ts'))