merge from master

giampaolo · Nov 14, 2017 · de7b7dc · de7b7dc
2 parents 99b757c + 845e57e
commit de7b7dc
Show file tree

Hide file tree

Showing 16 changed files with 86 additions and 26 deletions.
diff --git a/HISTORY.rst b/HISTORY.rst
@@ -5,6 +5,11 @@
 
 *XXXX-XX-XX*
 
+**Enhancements**
+
+- 1173_: introduced PSUTIL_DEBUG environment variable which can be set in order
+  to print useful debug messages on stderr (useful in case of nasty errors).
+
 **Bug fixes**
 
 - 1152_: [Windows] disk_io_counters() may return an empty dict.

diff --git a/Makefile b/Makefile
@@ -26,7 +26,7 @@ DEPS = \
 
 # In not in a virtualenv, add --user options for install commands.
 INSTALL_OPTS = `$(PYTHON) -c "import sys; print('' if hasattr(sys, 'real_prefix') else '--user')"`
-TEST_PREFIX = PYTHONWARNINGS=all PSUTIL_TESTING=1
+TEST_PREFIX = PYTHONWARNINGS=all PSUTIL_TESTING=1 PSUTIL_DEBUG=1
 
 all: test
 

diff --git a/appveyor.yml b/appveyor.yml
@@ -83,7 +83,7 @@ build: off
 
 test_script:
   - "%WITH_COMPILER% %PYTHON%/python -V"
-  - "set PYTHONWARNINGS=all && set PSUTIL_TESTING=1 && %WITH_COMPILER% %PYTHON%/python psutil/tests/__main__.py"
+  - "set PYTHONWARNINGS=all && set PSUTIL_TESTING=1 && set PSUTIL_DEBUG=1 && %WITH_COMPILER% %PYTHON%/python psutil/tests/__main__.py"
 
 after_test:
   - "%WITH_COMPILER% %PYTHON%/python setup.py bdist_wheel"

diff --git a/docs/index.rst b/docs/index.rst
@@ -2300,6 +2300,29 @@ Constants
 
 ----
 
+Debug mode
+==========
+
+In case you bump into nasty errors which look like being psutil's fault you may
+want to run psutil in debug mode. psutil may (or may not) print some useful
+message on stderr before crashing with an exception
+(see `original motivation <https://github.com/giampaolo/psutil/issues/1173>`__).
+To enable debug mode on UNIX:
+
+.. code-block:: bash
+
+    PSUTIL_DEBUG=1 python script.py
+
+On Windows:
+
+.. code-block:: bat
+
+    set PSUTIL_DEBUG=1 && C:\python36\python.exe script.py
+
+.. versionadded:: 5.4.2
+
+----
+
 Unicode
 =======
 

diff --git a/psutil/_psutil_bsd.c b/psutil/_psutil_bsd.c
@@ -475,7 +475,7 @@ psutil_proc_open_files(PyObject *self, PyObject *args) {
     errno = 0;
     freep = kinfo_getfile(pid, &cnt);
     if (freep == NULL) {
-        psutil_raise_for_pid(pid, "kinfo_getfile() failed");
+        psutil_raise_for_pid(pid, "kinfo_getfile()");
         goto error;
     }
 

diff --git a/psutil/_psutil_common.c b/psutil/_psutil_common.c
@@ -11,6 +11,7 @@
 
 
 // Global vars.
+int PSUTIL_DEBUG = 0;
 int PSUTIL_TESTING = 0;
 
 
@@ -77,11 +78,27 @@ psutil_set_testing(PyObject *self, PyObject *args) {
 }
 
 
+/*
+ * Print a debug message on stderr. No-op if PSUTIL_DEBUG env var is not set.
+ */
+void
+psutil_debug(const char* format, ...) {
+    va_list argptr;
+    va_start(argptr, format);
+    fprintf(stderr, "psutil-dubug> ");
+    vfprintf(stderr, format, argptr);
+    fprintf(stderr, "\n");
+    va_end(argptr);
+}
+
+
 /*
  * Called on module import on all platforms.
  */
 void
 psutil_setup(void) {
+    if (getenv("PSUTIL_DEBUG") != NULL)
+        PSUTIL_DEBUG = 1;
     if (getenv("PSUTIL_TESTING") != NULL)
         PSUTIL_TESTING = 1;
 }
diff --git a/psutil/_psutil_common.h b/psutil/_psutil_common.h
@@ -7,6 +7,7 @@
 #include <Python.h>
 
 extern int PSUTIL_TESTING;
+extern int PSUTIL_DEBUG;
 
 // a signaler for connections without an actual status
 static const int PSUTIL_CONN_NONE = 128;
@@ -20,4 +21,5 @@ PyObject* AccessDenied(char *msg);
 PyObject* NoSuchProcess(char *msg);
 
 PyObject* psutil_set_testing(PyObject *self, PyObject *args);
+void psutil_debug(const char* format, ...);
 void psutil_setup(void);
diff --git a/psutil/_psutil_osx.c b/psutil/_psutil_osx.c
@@ -266,7 +266,7 @@ psutil_proc_exe(PyObject *self, PyObject *args) {
         if (pid == 0)
             AccessDenied("");
         else
-            psutil_raise_for_pid(pid, "proc_pidpath() syscall failed");
+            psutil_raise_for_pid(pid, "proc_pidpath()");
         return NULL;
     }
     return PyUnicode_DecodeFSDefault(buf);
@@ -336,7 +336,7 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
 
     err = task_for_pid(mach_task_self(), (pid_t)pid, &task);
     if (err != KERN_SUCCESS) {
-        psutil_raise_for_pid(pid, "task_for_pid() failed");
+        psutil_raise_for_pid(pid, "task_for_pid()");
         goto error;
     }
 
@@ -376,8 +376,7 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
             errno = 0;
             proc_regionfilename((pid_t)pid, address, buf, sizeof(buf));
             if ((errno != 0) || ((sizeof(buf)) <= 0)) {
-                psutil_raise_for_pid(
-                    pid, "proc_regionfilename() syscall failed");
+                psutil_raise_for_pid(pid, "proc_regionfilename()");
                 goto error;
             }
 
@@ -1147,7 +1146,8 @@ psutil_proc_open_files(PyObject *self, PyObject *args) {
                     continue;
                 }
                 else {
-                    psutil_raise_for_pid(pid, "proc_pidinfo() syscall failed");
+                    psutil_raise_for_pid(
+                        pid, "proc_pidinfo(PROC_PIDFDVNODEPATHINFO)");
                     goto error;
                 }
             }
@@ -1259,7 +1259,8 @@ psutil_proc_connections(PyObject *self, PyObject *args) {
                     continue;
                 }
                 else {
-                    psutil_raise_for_pid(pid, "proc_pidinfo() syscall failed");
+                    psutil_raise_for_pid(
+                        pid, "proc_pidinfo(PROC_PIDFDSOCKETINFO)");
                     goto error;
                 }
             }

diff --git a/psutil/_psutil_posix.c b/psutil/_psutil_posix.c
@@ -112,16 +112,21 @@ psutil_pid_exists(long pid) {
  * This will always set a Python exception and return NULL.
  */
 int
-psutil_raise_for_pid(long pid, char *msg) {
+psutil_raise_for_pid(long pid, char *syscall_name) {
     // Set exception to AccessDenied if pid exists else NoSuchProcess.
     if (errno != 0) {
+        // Unlikely we get here.
         PyErr_SetFromErrno(PyExc_OSError);
         return 0;
     }
-    if (psutil_pid_exists(pid) == 0)
+    else if (psutil_pid_exists(pid) == 0) {
+        psutil_debug("%s syscall failed and PID %i no longer exists; "
+                     "assume NoSuchProcess", syscall_name, pid);
         NoSuchProcess("");
-    else
-        PyErr_SetString(PyExc_RuntimeError, msg);
+    }
+    else {
+        PyErr_Format(PyExc_RuntimeError, "%s syscall failed", syscall_name);
+    }
     return 0;
 }
 

diff --git a/psutil/_psutil_windows.c b/psutil/_psutil_windows.c
@@ -335,12 +335,14 @@ psutil_proc_kill(PyObject *self, PyObject *args) {
     if (! PyArg_ParseTuple(args, "l", &pid))
         return NULL;
     if (pid == 0)
-        return AccessDenied("");
+        return AccessDenied();
 
     hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
     if (hProcess == NULL) {
         if (GetLastError() == ERROR_INVALID_PARAMETER) {
             // see https://github.com/giampaolo/psutil/issues/24
+            psutil_debug("OpenProcess -> ERROR_INVALID_PARAMETER turned "
+                         "into NoSuchProcess");
             NoSuchProcess("");
         }
         else {
@@ -379,7 +381,7 @@ psutil_proc_wait(PyObject *self, PyObject *args) {
     if (! PyArg_ParseTuple(args, "ll", &pid, &timeout))
         return NULL;
     if (pid == 0)
-        return AccessDenied("");
+        return AccessDenied();
 
     hProcess = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,
                            FALSE, pid);
@@ -1067,7 +1069,7 @@ psutil_proc_suspend_or_resume(DWORD pid, int suspend) {
     THREADENTRY32  te32 = {0};
 
     if (pid == 0) {
-        AccessDenied("");
+        AccessDenied();
         return FALSE;
     }
 
@@ -1169,7 +1171,7 @@ psutil_proc_threads(PyObject *self, PyObject *args) {
     if (pid == 0) {
         // raise AD instead of returning 0 as procexp is able to
         // retrieve useful information somehow
-        AccessDenied("");
+        AccessDenied();
         goto error;
     }
 
@@ -2405,10 +2407,14 @@ psutil_disk_io_counters(PyObject *self, PyObject *args) {
                 //      1364/job/ascpdi271b06jle3
                 // Assume it means we're dealing with some exotic disk
                 // and go on.
+                psutil_debug("DeviceIoControl -> ERROR_INVALID_FUNCTION; "
+                             "ignore PhysicalDrive%i", devNum);
                 goto next;
             }
             else if (GetLastError() == ERROR_NOT_SUPPORTED) {
                 // Again, let's assume we're dealing with some exotic disk.
+                psutil_debug("DeviceIoControl -> ERROR_NOT_SUPPORTED; "
+                             "ignore PhysicalDrive%i", devNum);
                 goto next;
             }
             // XXX: it seems we should also catch ERROR_INVALID_PARAMETER:
@@ -2423,7 +2429,7 @@ psutil_disk_io_counters(PyObject *self, PyObject *args) {
             goto error;
         }
 
-        sprintf_s(szDeviceDisplay, MAX_PATH, "PhysicalDrive%d", devNum);
+        sprintf_s(szDeviceDisplay, MAX_PATH, "PhysicalDrive%i", devNum);
         py_tuple = Py_BuildValue(
             "(IILLKK)",
             diskPerformance.ReadCount,

diff --git a/psutil/arch/freebsd/proc_socks.c b/psutil/arch/freebsd/proc_socks.c
@@ -212,7 +212,7 @@ psutil_proc_connections(PyObject *self, PyObject *args) {
     errno = 0;
     freep = kinfo_getfile(pid, &cnt);
     if (freep == NULL) {
-        psutil_raise_for_pid(pid, "kinfo_getfile() failed");
+        psutil_raise_for_pid(pid, "kinfo_getfile()");
         goto error;
     }
 

diff --git a/psutil/arch/freebsd/specific.c b/psutil/arch/freebsd/specific.c
@@ -548,7 +548,7 @@ psutil_proc_cwd(PyObject *self, PyObject *args) {
     errno = 0;
     freep = kinfo_getfile(pid, &cnt);
     if (freep == NULL) {
-        psutil_raise_for_pid(pid, "kinfo_getfile() failed");
+        psutil_raise_for_pid(pid, "kinfo_getfile()");
         goto error;
     }
 
@@ -597,7 +597,7 @@ psutil_proc_num_fds(PyObject *self, PyObject *args) {
     errno = 0;
     freep = kinfo_getfile(pid, &cnt);
     if (freep == NULL) {
-        psutil_raise_for_pid(pid, "kinfo_getfile() failed");
+        psutil_raise_for_pid(pid, "kinfo_getfile()");
         return NULL;
     }
     free(freep);
@@ -765,7 +765,7 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
     errno = 0;
     freep = kinfo_getvmmap(pid, &cnt);
     if (freep == NULL) {
-        psutil_raise_for_pid(pid, "kinfo_getvmmap() failed");
+        psutil_raise_for_pid(pid, "kinfo_getvmmap()");
         goto error;
     }
     for (i = 0; i < cnt; i++) {

diff --git a/psutil/arch/netbsd/specific.c b/psutil/arch/netbsd/specific.c
@@ -502,7 +502,7 @@ psutil_proc_num_fds(PyObject *self, PyObject *args) {
     errno = 0;
     freep = kinfo_getfile(pid, &cnt);
     if (freep == NULL) {
-        psutil_raise_for_pid(pid, "kinfo_getfile() failed");
+        psutil_raise_for_pid(pid, "kinfo_getfile()");
         return NULL;
     }
     free(freep);

diff --git a/psutil/arch/openbsd/specific.c b/psutil/arch/openbsd/specific.c
@@ -404,7 +404,7 @@ psutil_proc_num_fds(PyObject *self, PyObject *args) {
     errno = 0;
     freep = kinfo_getfile(pid, &cnt);
     if (freep == NULL) {
-        psutil_raise_for_pid(pid, "kinfo_getfile() failed");
+        psutil_raise_for_pid(pid, "kinfo_getfile()");
         return NULL;
     }
     free(freep);
@@ -509,7 +509,7 @@ psutil_proc_connections(PyObject *self, PyObject *args) {
     errno = 0;
     freep = kinfo_getfile(pid, &cnt);
     if (freep == NULL) {
-        psutil_raise_for_pid(pid, "kinfo_getfile() failed");
+        psutil_raise_for_pid(pid, "kinfo_getfile()");
         goto error;
     }
 

diff --git a/psutil/arch/osx/process_info.c b/psutil/arch/osx/process_info.c
@@ -354,7 +354,7 @@ psutil_proc_pidinfo(long pid, int flavor, uint64_t arg, void *pti, int size) {
     errno = 0;
     int ret = proc_pidinfo((int)pid, flavor, arg, pti, size);
     if ((ret <= 0) || ((unsigned long)ret < sizeof(pti))) {
-        psutil_raise_for_pid(pid, "proc_pidinfo() syscall failed");
+        psutil_raise_for_pid(pid, "proc_pidinfo()");
         return 0;
     }
     return ret;

diff --git a/scripts/internal/winmake.py b/scripts/internal/winmake.py
@@ -177,6 +177,7 @@ def recursive_rm(*patterns):
 def test_setup():
     os.environ['PYTHONWARNINGS'] = 'all'
     os.environ['PSUTIL_TESTING'] = '1'
+    os.environ['PSUTIL_DEBUG'] = '1'
 
 
 # ===================================================================