OSX - wrapper around task_for_pid() (#1296)

(OSX) wrapper around task_for_pid()
fix #1181, fix #1209, fix #1291

4 files changed, 79 insertions, 35 deletions

diff --git a/HISTORY.rst b/HISTORY.rst
index b07a0df8..5854fa9f 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -1,5 +1,15 @@
 *Bug tracker at https://github.com/giampaolo/psutil/issues*
 
+5.4.7
+=====
+
+XXXX-XX-XX
+
+**Bug fixes**
+
+- 1209_: [OSX] Process.memory_maps() may fail with EINVAL due to poor
+  task_for_pid() syscall. AccessDenied is now raised instead.
+
 5.4.6
 =====
 
diff --git a/psutil/_psosx.py b/psutil/_psosx.py
index 193f63e0..38f4066e 100644
--- a/psutil/_psosx.py
+++ b/psutil/_psosx.py
@@ -334,6 +334,8 @@ def wrap_exceptions(fun):
             if err.errno in (errno.EPERM, errno.EACCES):
                 raise AccessDenied(self.pid, self._name)
             raise
+        except cext.ZombieProcessError:
+            raise ZombieProcess(self.pid, self._name, self._ppid)
     return wrapper
 
 
@@ -557,8 +559,7 @@ class Process(object):
 
     @wrap_exceptions
     def threads(self):
-        with catch_zombie(self):
-            rawlist = cext.proc_threads(self.pid)
+        rawlist = cext.proc_threads(self.pid)
         retlist = []
         for thread_id, utime, stime in rawlist:
             ntuple = _common.pthread(thread_id, utime, stime)
@@ -567,5 +568,4 @@ class Process(object):
 
     @wrap_exceptions
     def memory_maps(self):
-        with catch_zombie(self):
-            return cext.proc_memory_maps(self.pid)
+        return cext.proc_memory_maps(self.pid)
diff --git a/psutil/_psutil_osx.c b/psutil/_psutil_osx.c
index 55dd64ca..19508723 100644
--- a/psutil/_psutil_osx.c
+++ b/psutil/_psutil_osx.c
@@ -48,6 +48,8 @@
 
 #define PSUTIL_TV2DOUBLE(t) ((t).tv_sec + (t).tv_usec / 1000000.0)
 
+static PyObject *ZombieProcessError;
+
 
 /*
  * A wrapper around host_statistics() invoked with HOST_VM_INFO.
@@ -72,6 +74,59 @@ psutil_sys_vminfo(vm_statistics_data_t *vmstat) {
 
 
 /*
+ * Return 1 if pid refers to a zombie process else 0.
+ */
+int
+psutil_is_zombie(long pid)
+{
+    struct kinfo_proc kp;
+
+    if (psutil_get_kinfo_proc(pid, &kp) == -1)
+        return 0;
+    return (kp.kp_proc.p_stat == SZOMB) ? 1 : 0;
+}
+
+
+/*
+ * A wrapper around task_for_pid() which sucks big time:
+ * - it's not documented
+ * - errno is set only sometimes
+ * - sometimes errno is ENOENT (?!?)
+ * - for PIDs != getpid() or PIDs which are not members of the procmod
+ *   it requires root
+ * As such we can only guess what the heck went wrong and fail either
+ * with NoSuchProcess, ZombieProcessError or giveup with AccessDenied.
+ * Here's some history:
+ * https://github.com/giampaolo/psutil/issues/1181
+ * https://github.com/giampaolo/psutil/issues/1209
+ * https://github.com/giampaolo/psutil/issues/1291#issuecomment-396062519
+ */
+int
+psutil_task_for_pid(long pid, mach_port_t *task)
+{
+    // See: https://github.com/giampaolo/psutil/issues/1181
+    kern_return_t err = KERN_SUCCESS;
+
+    err = task_for_pid(mach_task_self(), (pid_t)pid, task);
+    if (err != KERN_SUCCESS) {
+        if (psutil_pid_exists(pid) == 0)
+            NoSuchProcess("task_for_pid() failed");
+        else if (psutil_is_zombie(pid) == 1)
+            PyErr_SetString(ZombieProcessError, "task_for_pid() failed");
+        else {
+            psutil_debug(
+                "task_for_pid() failed (pid=%ld, err=%i, errno=%i, msg='%s'); "
+                "setting AccessDenied()",
+                pid, err, errno, mach_error_string(err));
+            AccessDenied("task_for_pid() failed");
+        }
+        return 1;
+    }
+    return 0;
+}
+
+
+/*
  * Return a Python list of all the PIDs running on the system.
  */
 static PyObject *
@@ -336,20 +391,8 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
     if (! PyArg_ParseTuple(args, "l", &pid))
         goto error;
 
-    err = task_for_pid(mach_task_self(), (pid_t)pid, &task);
-    if (err != KERN_SUCCESS) {
-        if ((err == 5) && (errno == ENOENT)) {
-            // See: https://github.com/giampaolo/psutil/issues/1181
-            psutil_debug("task_for_pid(MACH_PORT_NULL) failed; err=%i, "
-                         "errno=%i, msg='%s'\n", err, errno,
-                         mach_error_string(err));
-            AccessDenied("");
-        }
-        else {
-            psutil_raise_for_pid(pid, "task_for_pid(MACH_PORT_NULL)");
-        }
+    if (psutil_task_for_pid(pid, &task) != 0)
         goto error;
-    }
 
     while (1) {
         py_tuple = NULL;
@@ -560,7 +603,6 @@ psutil_in_shared_region(mach_vm_address_t addr, cpu_type_t type) {
 static PyObject *
 psutil_proc_memory_uss(PyObject *self, PyObject *args) {
     long pid;
-    int err;
     size_t len;
     cpu_type_t cpu_type;
     size_t private_pages = 0;
@@ -576,14 +618,8 @@ psutil_proc_memory_uss(PyObject *self, PyObject *args) {
     if (! PyArg_ParseTuple(args, "l", &pid))
         return NULL;
 
-    err = task_for_pid(mach_task_self(), (pid_t)pid, &task);
-    if (err != KERN_SUCCESS) {
-        if (psutil_pid_exists(pid) == 0)
-            NoSuchProcess("");
-        else
-            AccessDenied("");
+    if (psutil_task_for_pid(pid, &task) != 0)
         return NULL;
-    }
 
     len = sizeof(cpu_type);
     if (sysctlbyname("sysctl.proc_cputype", &cpu_type, &len, NULL, 0) != 0)
@@ -1018,19 +1054,11 @@ psutil_proc_threads(PyObject *self, PyObject *args) {
     if (py_retlist == NULL)
         return NULL;
 
-    // the argument passed should be a process id
     if (! PyArg_ParseTuple(args, "l", &pid))
         goto error;
 
-    // task_for_pid() requires root privileges
-    err = task_for_pid(mach_task_self(), (pid_t)pid, &task);
-    if (err != KERN_SUCCESS) {
-        if (psutil_pid_exists(pid) == 0)
-            NoSuchProcess("");
-        else
-            AccessDenied("");
+    if (psutil_task_for_pid(pid, &task) != 0)
         goto error;
-    }
 
     info_count = TASK_BASIC_INFO_COUNT;
     err = task_info(task, TASK_BASIC_INFO, (task_info_t)&tasks_info,
@@ -2036,6 +2064,12 @@ init_psutil_osx(void)
     PyModule_AddIntConstant(module, "TCPS_TIME_WAIT", TCPS_TIME_WAIT);
     PyModule_AddIntConstant(module, "PSUTIL_CONN_NONE", PSUTIL_CONN_NONE);
 
+    // Exception.
+    ZombieProcessError = PyErr_NewException(
+        "_psutil_osx.ZombieProcessError", NULL, NULL);
+    Py_INCREF(ZombieProcessError);
+    PyModule_AddObject(module, "ZombieProcessError", ZombieProcessError);
+
     psutil_setup();
 
     if (module == NULL)
diff --git a/psutil/tests/__main__.py b/psutil/tests/__main__.py
index 62fe0742..36554a12 100755
--- a/psutil/tests/__main__.py
+++ b/psutil/tests/__main__.py
@@ -13,7 +13,6 @@ $ python -m psutil.tests
 import contextlib
 import optparse
 import os
-import ssl
 import sys
 import tempfile
 try:
@@ -38,6 +37,7 @@ def install_pip():
     try:
         import pip  # NOQA
     except ImportError:
+        import ssl
         f = tempfile.NamedTemporaryFile(suffix='.py')
         with contextlib.closing(f):
             print("downloading %s to %s" % (GET_PIP_URL, f.name))