openstack/swift
Code Issues Proposed changes

Ensure sys.exit called in fork_child after exception

Browse Source 
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
This commit is contained in:
Jay S. Bryant 2014-09-30 15:08:59 -05:00 committed by John Dickinson
parent 702d88bc89
commit 301a96f664
2 changed files with 18 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
swift/obj/auditor.py
View File

@ -257,7 +257,11 @@ class ObjectAuditor(Daemon):
signal.signal(signal.SIGTERM, signal.SIG_DFL)
if zero_byte_fps:
kwargs['zero_byte_fps'] = self.conf_zero_byte_fps
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):

12
test/unit/obj/test_auditor.py
View File

@ -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'))