799 onshot / win: no longer store the handle in python; I am now sure this is slower than using OpenProcess/CloseHandle in C

4 files changed, 97 insertions, 116 deletions

diff --git a/psutil/_psutil_windows.c b/psutil/_psutil_windows.c
index 5a023050..00bd2234 100644
--- a/psutil/_psutil_windows.c
+++ b/psutil/_psutil_windows.c
@@ -409,14 +409,17 @@ psutil_proc_wait(PyObject *self, PyObject *args) {
 static PyObject *
 psutil_proc_cpu_times(PyObject *self, PyObject *args) {
     long        pid;
-    unsigned long handle;
+    HANDLE      hProcess;
     FILETIME    ftCreate, ftExit, ftKernel, ftUser;
 
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
         return NULL;
 
-    if (! GetProcessTimes(
-            (HANDLE)handle, &ftCreate, &ftExit, &ftKernel, &ftUser)) {
+    hProcess = psutil_handle_from_pid(pid);
+    if (hProcess == NULL)
+        return NULL;
+    if (! GetProcessTimes(hProcess, &ftCreate, &ftExit, &ftKernel, &ftUser)) {
+        CloseHandle(hProcess);
         if (GetLastError() == ERROR_ACCESS_DENIED) {
             // usually means the process has died so we throw a NoSuchProcess
             // here
@@ -428,6 +431,8 @@ psutil_proc_cpu_times(PyObject *self, PyObject *args) {
         }
     }
 
+    CloseHandle(hProcess);
+
     /*
      * User and kernel times are represented as a FILETIME structure
      * wich contains a 64-bit value representing the number of
@@ -711,7 +716,7 @@ psutil_proc_name(PyObject *self, PyObject *args) {
  */
 static PyObject *
 psutil_proc_memory_info(PyObject *self, PyObject *args) {
-    unsigned long handle;
+    HANDLE hProcess;
     DWORD pid;
 #if (_WIN32_WINNT >= 0x0501)  // Windows XP with SP2
     PROCESS_MEMORY_COUNTERS_EX cnt;
@@ -720,11 +725,16 @@ psutil_proc_memory_info(PyObject *self, PyObject *args) {
 #endif
     SIZE_T private = 0;
 
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
+        return NULL;
+
+    hProcess = psutil_handle_from_pid(pid);
+    if (NULL == hProcess)
         return NULL;
 
-    if (! GetProcessMemoryInfo(
-            (HANDLE)handle, (PPROCESS_MEMORY_COUNTERS)&cnt, sizeof(cnt))) {
+    if (! GetProcessMemoryInfo(hProcess, (PPROCESS_MEMORY_COUNTERS)&cnt,
+                               sizeof(cnt))) {
+        CloseHandle(hProcess);
         return PyErr_SetFromWindowsErr(0);
     }
 
@@ -732,6 +742,8 @@ psutil_proc_memory_info(PyObject *self, PyObject *args) {
     private = cnt.PrivateUsage;
 #endif
 
+    CloseHandle(hProcess);
+
     // PROCESS_MEMORY_COUNTERS values are defined as SIZE_T which on 64bits
     // is an (unsigned long long) and on 32bits is an (unsigned int).
     // "_WIN64" is defined if we're running a 64bit Python interpreter not
@@ -1302,14 +1314,22 @@ psutil_proc_username(PyObject *self, PyObject *args) {
     ULONG domainNameSize;
     SID_NAME_USE nameUse;
     PTSTR fullName;
-    unsigned long handle;
     PyObject *py_unicode;
 
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
+        return NULL;
+
+    processHandle = psutil_handle_from_pid_waccess(
+        pid, PROCESS_QUERY_INFORMATION);
+    if (processHandle == NULL)
         return NULL;
 
-    if (!OpenProcessToken((HANDLE)handle, TOKEN_QUERY, &tokenHandle))
+    if (!OpenProcessToken(processHandle, TOKEN_QUERY, &tokenHandle)) {
+        CloseHandle(processHandle);
         return PyErr_SetFromWindowsErr(0);
+    }
+
+    CloseHandle(processHandle);
 
     // Get the user SID.
 
@@ -1933,11 +1953,15 @@ static PyObject *
 psutil_proc_priority_get(PyObject *self, PyObject *args) {
     long pid;
     DWORD priority;
-    unsigned long handle;
+    HANDLE hProcess;
 
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
+        return NULL;
+    hProcess = psutil_handle_from_pid(pid);
+    if (hProcess == NULL)
         return NULL;
-    priority = GetPriorityClass((HANDLE)handle);
+    priority = GetPriorityClass(hProcess);
+    CloseHandle(hProcess);
     if (priority == 0) {
         PyErr_SetFromWindowsErr(0);
         return NULL;
@@ -1979,23 +2003,27 @@ psutil_proc_priority_set(PyObject *self, PyObject *args) {
 static PyObject *
 psutil_proc_io_priority_get(PyObject *self, PyObject *args) {
     long pid;
-    unsigned long handle;
+    HANDLE hProcess;
     PULONG IoPriority;
 
     _NtQueryInformationProcess NtQueryInformationProcess =
         (_NtQueryInformationProcess)GetProcAddress(
             GetModuleHandleA("ntdll.dll"), "NtQueryInformationProcess");
 
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
+        return NULL;
+    hProcess = psutil_handle_from_pid(pid);
+    if (hProcess == NULL)
         return NULL;
 
     NtQueryInformationProcess(
-        (HANDLE)handle,
+        hProcess,
         ProcessIoPriority,
         &IoPriority,
         sizeof(ULONG),
         NULL
     );
+    CloseHandle(hProcess);
     return Py_BuildValue("i", IoPriority);
 }
 
@@ -2044,13 +2072,19 @@ psutil_proc_io_priority_set(PyObject *self, PyObject *args) {
 static PyObject *
 psutil_proc_io_counters(PyObject *self, PyObject *args) {
     DWORD pid;
-    unsigned long handle;
+    HANDLE hProcess;
     IO_COUNTERS IoCounters;
 
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
+        return NULL;
+    hProcess = psutil_handle_from_pid(pid);
+    if (NULL == hProcess)
         return NULL;
-    if (! GetProcessIoCounters((HANDLE)handle, &IoCounters))
+    if (! GetProcessIoCounters(hProcess, &IoCounters)) {
+        CloseHandle(hProcess);
         return PyErr_SetFromWindowsErr(0);
+    }
+    CloseHandle(hProcess);
     return Py_BuildValue("(KKKK)",
                          IoCounters.ReadOperationCount,
                          IoCounters.WriteOperationCount,
@@ -2065,17 +2099,22 @@ psutil_proc_io_counters(PyObject *self, PyObject *args) {
 static PyObject *
 psutil_proc_cpu_affinity_get(PyObject *self, PyObject *args) {
     DWORD pid;
-    unsigned long handle;
+    HANDLE hProcess;
     DWORD_PTR proc_mask;
     DWORD_PTR system_mask;
 
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
+        return NULL;
+    hProcess = psutil_handle_from_pid(pid);
+    if (hProcess == NULL) {
         return NULL;
-    if (GetProcessAffinityMask(
-            (HANDLE)handle, &proc_mask, &system_mask) == 0) {
+    }
+    if (GetProcessAffinityMask(hProcess, &proc_mask, &system_mask) == 0) {
+        CloseHandle(hProcess);
         return PyErr_SetFromWindowsErr(0);
     }
 
+    CloseHandle(hProcess);
 #ifdef _WIN64
     return Py_BuildValue("K", (unsigned long long)proc_mask);
 #else
@@ -2665,14 +2704,19 @@ error:
 static PyObject *
 psutil_proc_num_handles(PyObject *self, PyObject *args) {
     DWORD pid;
-    unsigned long handle;
+    HANDLE hProcess;
     DWORD handleCount;
 
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
+        return NULL;
+    hProcess = psutil_handle_from_pid(pid);
+    if (NULL == hProcess)
         return NULL;
-    if (! GetProcessHandleCount((HANDLE)handle, &handleCount)) {
+    if (! GetProcessHandleCount(hProcess, &handleCount)) {
+        CloseHandle(hProcess);
         return PyErr_SetFromWindowsErr(0);
     }
+    CloseHandle(hProcess);
     return Py_BuildValue("k", handleCount);
 }
 
@@ -2809,13 +2853,15 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
     CHAR mappedFileName[MAX_PATH];
     SYSTEM_INFO system_info;
     LPVOID maxAddr;
-    unsigned long handle;
     PyObject *py_retlist = PyList_New(0);
     PyObject *py_tuple = NULL;
 
     if (py_retlist == NULL)
         return NULL;
-    if (! PyArg_ParseTuple(args, "lk", &pid, &handle))
+    if (! PyArg_ParseTuple(args, "l", &pid))
+        goto error;
+    hProcess = psutil_handle_from_pid(pid);
+    if (NULL == hProcess)
         goto error;
 
     GetSystemInfo(&system_info);
@@ -2823,13 +2869,13 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
     baseAddress = NULL;
     previousAllocationBase = NULL;
 
-    while (VirtualQueryEx((HANDLE)handle, baseAddress, &basicInfo,
+    while (VirtualQueryEx(hProcess, baseAddress, &basicInfo,
                           sizeof(MEMORY_BASIC_INFORMATION)))
     {
         py_tuple = NULL;
         if (baseAddress > maxAddr)
             break;
-        if (GetMappedFileNameA((HANDLE)handle, baseAddress, mappedFileName,
+        if (GetMappedFileNameA(hProcess, baseAddress, mappedFileName,
                                sizeof(mappedFileName)))
         {
 #ifdef _WIN64
@@ -2855,11 +2901,14 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
         baseAddress = (PCHAR)baseAddress + basicInfo.RegionSize;
     }
 
+    CloseHandle(hProcess);
     return py_retlist;
 
 error:
     Py_XDECREF(py_tuple);
     Py_DECREF(py_retlist);
+    if (hProcess != NULL)
+        CloseHandle(hProcess);
     return NULL;
 }
 
@@ -3473,10 +3522,6 @@ PsutilMethods[] = {
     // --- windows API bindings
     {"win32_QueryDosDevice", psutil_win32_QueryDosDevice, METH_VARARGS,
      "QueryDosDevice binding"},
-    {"win32_OpenProcess", psutil_win32_OpenProcess, METH_VARARGS,
-     "Given a PID return a Python int which points to a process handle."},
-    {"win32_CloseHandle", psutil_win32_CloseHandle, METH_VARARGS,
-     "Given a Python int referencing a process handle it close the handle."},
 
     {NULL, NULL, 0, NULL}
 };
diff --git a/psutil/_pswindows.py b/psutil/_pswindows.py
index 105655f7..d956b91f 100644
--- a/psutil/_pswindows.py
+++ b/psutil/_pswindows.py
@@ -15,11 +15,11 @@ from . import _common
 from . import _psutil_windows as cext
 from ._common import conn_tmap
 from ._common import isfile_strict
-from ._common import memoize_when_activated
 from ._common import parse_environ_block
 from ._common import sockfam_to_enum
 from ._common import socktype_to_enum
 from ._common import usage_percent
+from ._common import memoize_when_activated
 from ._compat import long
 from ._compat import lru_cache
 from ._compat import PY3
@@ -570,54 +570,20 @@ def wrap_exceptions(fun):
 class Process(object):
     """Wrapper class around underlying C implementation."""
 
-    __slots__ = ["pid", "_name", "_ppid", "_inctx", "_handle"]
+    __slots__ = ["pid", "_name", "_ppid"]
 
     def __init__(self, pid):
         self.pid = pid
         self._name = None
         self._ppid = None
-        self._inctx = False
-        self._handle = None
 
     # --- oneshot() stuff
 
     def oneshot_enter(self):
-        self._inctx = True
         self.oneshot_info.cache_activate()
 
     def oneshot_exit(self):
-        self._inctx = False
         self.oneshot_info.cache_deactivate()
-        if self._handle is not None:
-            try:
-                cext.win32_CloseHandle(self._handle)
-            finally:
-                self._handle = None
-
-    def get_handle(self):
-        """Get a handle to this process.
-        If we're in oneshot() context returns the cached handle.
-        """
-        if self._inctx:
-            self._handle = self._handle or cext.win32_OpenProcess(self.pid)
-            return self._handle
-        else:
-            return cext.win32_OpenProcess(self.pid)
-
-    @contextlib.contextmanager
-    def handle_ctx(self):
-        """Get a handle to this process as a context manager.
-        If we're not in a oneshot() context close the handle
-        when exiting the "with" statement, else try return the
-        cached handle (if available) and don't close it when
-        exiting the "with" statement.
-        """
-        handle = self.get_handle()
-        try:
-            yield handle
-        finally:
-            if not self._inctx:
-                cext.win32_CloseHandle(handle)
 
     @memoize_when_activated
     def oneshot_info(self):
@@ -628,8 +594,6 @@ class Process(object):
         assert len(ret) == len(pinfo_map)
         return ret
 
-    # --- implementation
-
     @wrap_exceptions
     def name(self):
         """Return process name, which on Windows is always the final
@@ -681,8 +645,7 @@ class Process(object):
 
     def _get_raw_meminfo(self):
         try:
-            with self.handle_ctx() as handle:
-                return cext.proc_memory_info(self.pid, handle)
+            return cext.proc_memory_info(self.pid)
         except OSError as err:
             if err.errno in ACCESS_DENIED_SET:
                 # TODO: the C ext can probably be refactored in order
@@ -720,8 +683,7 @@ class Process(object):
 
     def memory_maps(self):
         try:
-            with self.handle_ctx() as handle:
-                raw = cext.proc_memory_maps(self.pid, handle)
+            raw = cext.proc_memory_maps(self.pid)
         except OSError as err:
             # XXX - can't use wrap_exceptions decorator as we're
             # returning a generator; probably needs refactoring.
@@ -760,8 +722,7 @@ class Process(object):
     def username(self):
         if self.pid in (0, 4):
             return 'NT AUTHORITY\\SYSTEM'
-        with self.handle_ctx() as handle:
-            return cext.proc_username(self.pid, handle)
+        return cext.proc_username(self.pid)
 
     @wrap_exceptions
     def create_time(self):
@@ -791,8 +752,7 @@ class Process(object):
     @wrap_exceptions
     def cpu_times(self):
         try:
-            with self.handle_ctx() as handle:
-                user, system = cext.proc_cpu_times(self.pid, handle)
+            user, system = cext.proc_cpu_times(self.pid)
         except OSError as err:
             if err.errno in ACCESS_DENIED_SET:
                 info = self.oneshot_info()
@@ -845,8 +805,7 @@ class Process(object):
 
     @wrap_exceptions
     def nice_get(self):
-        with self.handle_ctx() as handle:
-            value = cext.proc_priority_get(self.pid, handle)
+        value = cext.proc_priority_get(self.pid)
         if enum is not None:
             value = Priority(value)
         return value
@@ -859,8 +818,7 @@ class Process(object):
     if hasattr(cext, "proc_io_priority_get"):
         @wrap_exceptions
         def ionice_get(self):
-            with self.handle_ctx() as handle:
-                return cext.proc_io_priority_get(self.pid, handle)
+            return cext.proc_io_priority_get(self.pid)
 
         @wrap_exceptions
         def ionice_set(self, value, _):
@@ -875,8 +833,7 @@ class Process(object):
     @wrap_exceptions
     def io_counters(self):
         try:
-            with self.handle_ctx() as handle:
-                ret = cext.proc_io_counters(self.pid, handle)
+            ret = cext.proc_io_counters(self.pid)
         except OSError as err:
             if err.errno in ACCESS_DENIED_SET:
                 info = self.oneshot_info()
@@ -902,8 +859,7 @@ class Process(object):
     def cpu_affinity_get(self):
         def from_bitmask(x):
             return [i for i in xrange(64) if (1 << i) & x]
-        with self.handle_ctx() as handle:
-            bitmask = cext.proc_cpu_affinity_get(self.pid, handle)
+        bitmask = cext.proc_cpu_affinity_get(self.pid)
         return from_bitmask(bitmask)
 
     @wrap_exceptions
@@ -934,8 +890,7 @@ class Process(object):
     @wrap_exceptions
     def num_handles(self):
         try:
-            with self.handle_ctx() as handle:
-                return cext.proc_num_handles(self.pid, handle)
+            return cext.proc_num_handles(self.pid)
         except OSError as err:
             if err.errno in ACCESS_DENIED_SET:
                 return self.oneshot_info()[pinfo_map['num_handles']]
diff --git a/psutil/arch/windows/process_info.c b/psutil/arch/windows/process_info.c
index 007f1ba2..e29f2161 100644
--- a/psutil/arch/windows/process_info.c
+++ b/psutil/arch/windows/process_info.c
@@ -68,23 +68,6 @@ psutil_handle_from_pid(DWORD pid) {
 
 
 /*
- * Given a PID return a Python int which points to its process handle.
- */
-PyObject *
-psutil_win32_OpenProcess(PyObject *self, PyObject *args) {
-    HANDLE handle;
-    long pid;
-
-    if (! PyArg_ParseTuple(args, "l", &pid))
-        return NULL;
-    handle = psutil_handle_from_pid(pid);
-    if (handle == NULL)
-        return NULL;
-    return HANDLE_TO_PYNUM(handle);
-}
-
-
-/*
  * Given a Python int referencing a process handle close the process handle.
  */
 PyObject *
diff --git a/scripts/internal/bench_oneshot.py b/scripts/internal/bench_oneshot.py
index 0316e34b..636d37ad 100755
--- a/scripts/internal/bench_oneshot.py
+++ b/scripts/internal/bench_oneshot.py
@@ -89,17 +89,15 @@ elif psutil.OSX:
     ]
 elif psutil.WINDOWS:
     names += [
-        'cpu_affinity',
-        'cpu_times',
-        'io_counters',
-        'ionice',
-        'memory_info',
-        # 'memory_maps',  # just makes things too slow
-        'nice',
         'num_ctx_switches',
+        'num_threads',
+        # dual implementation, called in case of AccessDenied
         'num_handles',
+        'cpu_times',
+        'create_time',
         'num_threads',
-        'username',
+        'io_counters',
+        'memory_info',
     ]
 
 names = sorted(set(names))