Skip to content

Commit

Permalink
Fix ng-killall by swallowing psutil exceptions in filter
Browse files Browse the repository at this point in the history
ng-killall will fail to kill nailguns if there is a process earlier in the process list that has attributes that can't be accessed.

This patch wraps ProcessManager.process_iter's filter with a exception swallow call which allows it to continue if the filter calls a psutil error causing method.

Testing Done:
Wrote unit tests exhibiting the behavior I was seeing and made them pass. Tried running ng-killall after the change and saw it work.

Bugs closed: 3840, 3869

Reviewed at https://rbcommons.com/s/twitter/r/4237/

closes pantsbuild#3840
closes pantsbuild#3869
  • Loading branch information
baroquebobcat committed Sep 14, 2016
1 parent 96e0187 commit 3e6e212
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
15 changes: 11 additions & 4 deletions src/python/pants/pantsd/process_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,17 @@ def _instance_from_process(self, process):
metadata_base_dir=self._metadata_base_dir)

def iter_processes(self, proc_filter=None):
proc_filter = proc_filter or (lambda x: True)
with swallow_psutil_exceptions():
for proc in (x for x in psutil.process_iter() if proc_filter(x)):
yield proc
"""Yields processes from psutil.process_iter with an optional filter and swallows psutil errors.
If a psutil exception is raised during execution of the filter, that process will not be
yielded but subsequent processes will. On the other hand, if psutil.process_iter raises
an exception, no more processes will be yielded.
"""
with swallow_psutil_exceptions(): # process_iter may raise
for proc in psutil.process_iter():
with swallow_psutil_exceptions(): # proc_filter may raise
if (proc_filter is None) or proc_filter(proc):
yield proc

def iter_instances(self, *args, **kwargs):
for item in self.iter_processes(*args, **kwargs):
Expand Down
33 changes: 33 additions & 0 deletions tests/python/pants_test/pantsd/test_process_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,39 @@ def test_iter_processes(self):
items = [item for item in self.pg.iter_processes()]
self.assertEqual(items, [5, 4, 3, 2, 1])

def test_iter_processes_filter_raises_psutil_exception(self):
"""If the filter triggers a psutil exception, skip the proc and continue."""
with mock.patch('psutil.process_iter', **PATCH_OPTS) as mock_process_iter:
def noop():
return True
def raises():
raise psutil.NoSuchProcess('a_test')

mock_process_iter.return_value = [
noop,
raises,
noop
]

items = [item for item in self.pg.iter_processes(proc_filter=lambda p: p())]
self.assertEqual([noop, noop], items)

def test_iter_processes_process_iter_raises_psutil_exception(self):
"""If psutil.process_iter raises the exception, silently stop iteration."""
def id_or_raise(o):
if isinstance(o, Exception):
raise o
else:
return o
with mock.patch('psutil.process_iter', **PATCH_OPTS) as mock_process_iter:
mock_process_iter.return_value= (id_or_raise(i)
for i in ['first',
psutil.NoSuchProcess('The Exception'),
'never seen'])

items = [item for item in self.pg.iter_processes()]
self.assertEqual(['first'], items)

def test_iter_processes_filtered(self):
with mock.patch('psutil.process_iter', **PATCH_OPTS) as mock_process_iter:
mock_process_iter.return_value = [5, 4, 3, 2, 1]
Expand Down

0 comments on commit 3e6e212

Please sign in to comment.