From 4a06a5403598eab336b25b13a6f603b46e15b183 Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Fri, 28 Oct 2016 10:35:16 +0200 Subject: [PATCH] #799 / win: pass handle also to memory_maps() and username() functions --- docs/index.rst | 86 ++++++++++---------- psutil/_psutil_windows.c | 27 ++---- psutil/_pswindows.py | 6 +- psutil/tests/test_windows.py | 131 ------------------------------ scripts/internal/bench_oneshot.py | 2 + 5 files changed, 56 insertions(+), 196 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index f3caada6a..5f01730b7 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -808,49 +808,49 @@ Process class The last column (speedup) shows an approximation of the speedup you can get if you call all the methods together (best case scenario). - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | Linux | Windows | OSX | BSD | SunOS | - +==============================+==============================+==============================+==============================+==========================+ - | :meth:`~Process.cpu_percent` | :meth:`~Process.cpu_percent` | :meth:`~Process.cpu_percent` | :meth:`~Process.cpu_percent` | :meth:`name` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`~Process.cpu_times` | :meth:`~Process.cpu_times` | :meth:`~Process.cpu_times` | :meth:`~Process.cpu_times` | :meth:`cmdline` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`create_time` | :meth:`io_counters()` | :meth:`memory_info` | :meth:`create_time` | :meth:`create_time` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`name` | :meth:`ionice` | :meth:`memory_percent` | :meth:`gids` | | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`ppid` | :meth:`memory_info` | :meth:`num_ctx_switches` | :meth:`io_counters` | :meth:`memory_info` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`status` | :meth:`nice` | :meth:`num_threads` | :meth:`name` | :meth:`memory_percent` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`terminal` | :meth:`num_ctx_switches` | | :meth:`memory_info` | :meth:`nice` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | | :meth:`num_handles` | :meth:`create_time` | :meth:`memory_percent` | :meth:`num_threads` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`gids` | :meth:`num_threads` | :meth:`gids` | :meth:`num_ctx_switches` | :meth:`ppid` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`num_ctx_switches` | | :meth:`name` | :meth:`ppid` | :meth:`status` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`num_threads` | | :meth:`ppid` | :meth:`status` | :meth:`terminal` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`uids` | | :meth:`status` | :meth:`terminal` | | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | :meth:`username` | | :meth:`terminal` | :meth:`uids` | :meth:`gids` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | | | :meth:`uids` | :meth:`username` | :meth:`uids` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | | | :meth:`username` | | :meth:`username` | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | | | | | | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - | *speedup: +2.5x* | *speedup: +1.8x* | *speedup: +1.9x* | *speedup: +2.0x* | | - +------------------------------+------------------------------+------------------------------+------------------------------+--------------------------+ - - .. versionadded:: 5.0.0 - - .. attribute:: pid - - The process PID. This is the only (read-only) attribute of the class. + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | Linux | Windows | OSX | BSD | SunOS | + +==============================+===============================+==============================+==============================+==========================+ + | :meth:`~Process.cpu_percent` | :meth:`~Process.cpu_percent` | :meth:`~Process.cpu_percent` | :meth:`~Process.cpu_percent` | :meth:`name` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`~Process.cpu_times` | :meth:`~Process.cpu_times` | :meth:`~Process.cpu_times` | :meth:`~Process.cpu_times` | :meth:`cmdline` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`create_time` | :meth:`io_counters()` | :meth:`memory_info` | :meth:`create_time` | :meth:`create_time` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`name` | :meth:`ionice` | :meth:`memory_percent` | :meth:`gids` | | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`ppid` | :meth:`memory_info` | :meth:`num_ctx_switches` | :meth:`io_counters` | :meth:`memory_info` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`status` | :meth:`nice` | :meth:`num_threads` | :meth:`name` | :meth:`memory_percent` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`terminal` | :meth:`memory_maps` | | :meth:`memory_info` | :meth:`nice` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | | :meth:`num_ctx_switches` | :meth:`create_time` | :meth:`memory_percent` | :meth:`num_threads` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`gids` | :meth:`num_handles` | :meth:`gids` | :meth:`num_ctx_switches` | :meth:`ppid` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`num_ctx_switches` | :meth:`num_threads` | :meth:`name` | :meth:`ppid` | :meth:`status` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`num_threads` | :meth:`username` | :meth:`ppid` | :meth:`status` | :meth:`terminal` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`uids` | | :meth:`status` | :meth:`terminal` | | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | :meth:`username` | | :meth:`terminal` | :meth:`uids` | :meth:`gids` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | | | :meth:`uids` | :meth:`username` | :meth:`uids` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | | | :meth:`username` | | :meth:`username` | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | | | | | | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + | *speedup: +2.5x* | *speedup: from +1.8x to +6.5* | *speedup: +1.9x* | *speedup: +2.0x* | | + +------------------------------+-------------------------------+------------------------------+------------------------------+--------------------------+ + + .. versionadded:: 5.0.0 + + .. attribute:: pid + + The process PID. This is the only (read-only) attribute of the class. .. method:: ppid() diff --git a/psutil/_psutil_windows.c b/psutil/_psutil_windows.c index af658bb00..e0abbeb83 100644 --- a/psutil/_psutil_windows.c +++ b/psutil/_psutil_windows.c @@ -1302,22 +1302,14 @@ 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, "l", &pid)) - return NULL; - - processHandle = psutil_handle_from_pid_waccess( - pid, PROCESS_QUERY_INFORMATION); - if (processHandle == NULL) + if (! PyArg_ParseTuple(args, "lk", &pid, &handle)) return NULL; - if (!OpenProcessToken(processHandle, TOKEN_QUERY, &tokenHandle)) { - CloseHandle(processHandle); + if (!OpenProcessToken((HANDLE)handle, TOKEN_QUERY, &tokenHandle)) return PyErr_SetFromWindowsErr(0); - } - - CloseHandle(processHandle); // Get the user SID. @@ -2817,15 +2809,13 @@ 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, "l", &pid)) - goto error; - hProcess = psutil_handle_from_pid(pid); - if (NULL == hProcess) + if (! PyArg_ParseTuple(args, "lk", &pid, &handle)) goto error; GetSystemInfo(&system_info); @@ -2833,13 +2823,13 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) { baseAddress = NULL; previousAllocationBase = NULL; - while (VirtualQueryEx(hProcess, baseAddress, &basicInfo, + while (VirtualQueryEx((HANDLE)handle, baseAddress, &basicInfo, sizeof(MEMORY_BASIC_INFORMATION))) { py_tuple = NULL; if (baseAddress > maxAddr) break; - if (GetMappedFileNameA(hProcess, baseAddress, mappedFileName, + if (GetMappedFileNameA((HANDLE)handle, baseAddress, mappedFileName, sizeof(mappedFileName))) { #ifdef _WIN64 @@ -2865,14 +2855,11 @@ 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; } diff --git a/psutil/_pswindows.py b/psutil/_pswindows.py index 99df53a55..105655f74 100644 --- a/psutil/_pswindows.py +++ b/psutil/_pswindows.py @@ -720,7 +720,8 @@ def memory_full_info(self): def memory_maps(self): try: - raw = cext.proc_memory_maps(self.pid) + with self.handle_ctx() as handle: + raw = cext.proc_memory_maps(self.pid, handle) except OSError as err: # XXX - can't use wrap_exceptions decorator as we're # returning a generator; probably needs refactoring. @@ -759,7 +760,8 @@ def wait(self, timeout=None): def username(self): if self.pid in (0, 4): return 'NT AUTHORITY\\SYSTEM' - return cext.proc_username(self.pid) + with self.handle_ctx() as handle: + return cext.proc_username(self.pid, handle) @wrap_exceptions def create_time(self): diff --git a/psutil/tests/test_windows.py b/psutil/tests/test_windows.py index 40a840861..802242b55 100755 --- a/psutil/tests/test_windows.py +++ b/psutil/tests/test_windows.py @@ -15,7 +15,6 @@ import subprocess import sys import time -import traceback try: import win32api # requires "pip install pypiwin32" / "make setup-dev-env" @@ -29,7 +28,6 @@ from psutil import WINDOWS from psutil._compat import basestring from psutil._compat import callable -from psutil._compat import long from psutil._compat import PY3 from psutil.tests import APPVEYOR from psutil.tests import get_test_subprocess @@ -359,15 +357,6 @@ class TestDualProcessImplementation(unittest.TestCase): https://github.com/giampaolo/psutil/issues/304 """ - fun_names = [ - # function name, tolerance - ('proc_cpu_times', 0.2), - ('proc_create_time', 0.5), - ('proc_num_handles', 1), # 1 because impl #1 opens a handle - ('proc_memory_info', 1024), # KB - ('proc_io_counters', 0), - ] - @classmethod def setUpClass(cls): cls.pid = get_test_subprocess().pid @@ -375,118 +364,6 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): reap_children() - - def test_all_procs(self): - from psutil._pswindows import pinfo_map - - def assert_ge_0(obj): - if isinstance(obj, (tuple, list)): - for value in obj: - self.assertGreaterEqual(value, 0, msg=obj) - elif isinstance(obj, (int, long, float)): - self.assertGreaterEqual(obj, 0) - else: - assert 0 # case not handled which needs to be fixed - - def compare_with_tolerance(ret1, ret2, tolerance): - if ret1 == ret2: - return - else: - if isinstance(ret2, (int, long, float)): - diff = abs(ret1 - ret2) - self.assertLessEqual(diff, tolerance) - elif isinstance(ret2, tuple): - for a, b in zip(ret1, ret2): - diff = abs(a - b) - self.assertLessEqual(diff, tolerance) - - failures = [] - for p in psutil.process_iter(): - try: - raw_info = cext.proc_info(p.pid) - except psutil.NoSuchProcess: - continue - assert_ge_0(raw_info) - - for name, tolerance in self.fun_names: - if name == 'proc_memory_info' and p.pid == os.getpid(): - continue - if name == 'proc_create_time' and p.pid in (0, 4): - continue - meth = wrap_exceptions(getattr(cext, name)) - try: - ret = meth(p.pid) - except (psutil.NoSuchProcess, psutil.AccessDenied): - continue - # compare values - try: - if name == 'proc_cpu_times': - compare_with_tolerance( - ret[0], raw_info[pinfo_map['user_time']], - tolerance) - compare_with_tolerance( - ret[1], raw_info[pinfo_map['kernel_time']], - tolerance) - elif name == 'proc_create_time': - compare_with_tolerance( - ret, raw_info[pinfo_map['create_time']], tolerance) - elif name == 'proc_num_handles': - compare_with_tolerance( - ret, raw_info[pinfo_map['num_handles']], tolerance) - elif name == 'proc_io_counters': - compare_with_tolerance( - ret[0], raw_info[pinfo_map['io_rcount']], - tolerance) - compare_with_tolerance( - ret[1], raw_info[pinfo_map['io_wcount']], - tolerance) - compare_with_tolerance( - ret[2], raw_info[pinfo_map['io_rbytes']], - tolerance) - compare_with_tolerance( - ret[3], raw_info[pinfo_map['io_wbytes']], - tolerance) - elif name == 'proc_memory_info': - compare_with_tolerance( - ret[0], raw_info[pinfo_map['num_page_faults']], - tolerance) - compare_with_tolerance( - ret[1], raw_info[pinfo_map['peak_wset']], - tolerance) - compare_with_tolerance( - ret[2], raw_info[pinfo_map['wset']], - tolerance) - compare_with_tolerance( - ret[3], raw_info[pinfo_map['peak_paged_pool']], - tolerance) - compare_with_tolerance( - ret[4], raw_info[pinfo_map['paged_pool']], - tolerance) - compare_with_tolerance( - ret[5], raw_info[pinfo_map['peak_non_paged_pool']], - tolerance) - compare_with_tolerance( - ret[6], raw_info[pinfo_map['non_paged_pool']], - tolerance) - compare_with_tolerance( - ret[7], raw_info[pinfo_map['pagefile']], - tolerance) - compare_with_tolerance( - ret[8], raw_info[pinfo_map['peak_pagefile']], - tolerance) - compare_with_tolerance( - ret[9], raw_info[pinfo_map['mem_private']], - tolerance) - except AssertionError: - trace = traceback.format_exc() - msg = '%s\npid=%s, method=%r, ret_1=%r, ret_2=%r' % ( - trace, p.pid, name, ret, raw_info) - failures.append(msg) - break - - if failures: - self.fail('\n\n'.join(failures)) - # --- # same tests as above but mimicks the AccessDenied failure of # the first (fast) method failing with AD. @@ -549,14 +426,6 @@ def test_num_handles(self): psutil.Process(self.pid).num_handles() == num_handles assert fun.called - def test_zombies(self): - # test that NPS is raised by the 2nd implementation in case a - # process no longer exists - ZOMBIE_PID = max(psutil.pids()) + 5000 - for name, _ in self.fun_names: - meth = wrap_exceptions(getattr(cext, name)) - self.assertRaises(psutil.NoSuchProcess, meth, ZOMBIE_PID) - @unittest.skipUnless(WINDOWS, "WINDOWS only") class RemoteProcessTestCase(unittest.TestCase): diff --git a/scripts/internal/bench_oneshot.py b/scripts/internal/bench_oneshot.py index 5ab2266ef..0316e34bf 100755 --- a/scripts/internal/bench_oneshot.py +++ b/scripts/internal/bench_oneshot.py @@ -94,10 +94,12 @@ 'io_counters', 'ionice', 'memory_info', + # 'memory_maps', # just makes things too slow 'nice', 'num_ctx_switches', 'num_handles', 'num_threads', + 'username', ] names = sorted(set(names))