baweaver / psutil

Automatically exported from code.google.com/p/psutil
Other
0 stars 0 forks source link

Resolve AccessDenied exception problems occurring most of the times on Windows #304

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago

This is similar to issue 297 but for Windows.

On Windows we use native APIs to extract process information such as CPU times, 
memory info etc.
Such APIs systematically fail with an AccessDenied exception for any process 
owned by NT AUTHORITY SYSTEM user (system processes, typically) imposing a 
severe usability issue for those who want to inspect processes not owned by the 
current user (e.g. a task manager like app).
Example:

PID   NAME                 RSS MEM
0     System Idle Process  0
4     System               ACCESS DENIED
172   conhost.exe          3723264
248   svchost.exe          ACCESS DENIED
284   smss.exe             ACCESS DENIED
320   svchost.exe          ACCESS DENIED
328   TSVNCache.exe        7729152
376   csrss.exe            ACCESS DENIED
408   wininit.exe          ACCESS DENIED
424   csrss.exe            ACCESS DENIED
464   winlogon.exe         ACCESS DENIED
512   services.exe         ACCESS DENIED
520   lsass.exe            ACCESS DENIED
528   lsm.exe              ACCESS DENIED
564   python.exe           8675328
572   procexp64.exe        18870272
628   svchost.exe          ACCESS DENIED
688   VBoxService.exe      ACCESS DENIED
728   svchost.exe          ACCESS DENIED
796   explorer.exe         46014464
816   svchost.exe          ACCESS DENIED
856   svchost.exe          ACCESS DENIED
900   svchost.exe          ACCESS DENIED
968   svchost.exe          ACCESS DENIED
1064  dllhost.exe          5214208
1280  cmd.exe              6815744
1296  svchost.exe          ACCESS DENIED
1600  python.exe           5832704
1632  svchost.exe          ACCESS DENIED
1772  taskhost.exe         5554176
1840  dwm.exe              3375104
1852  TortoiseUDiff.exe    4263936
1952  VBoxTray.exe         3039232
2036  procexp.exe          5611520

I realized that a considerable amount of process information which we currently 
extract by using documented Windows APIs is also stored in the 
SYSTEM_PROCESS_INFORMATION structure, which can be obtained via 
NtQuerySystemInformation():
http://undocumented.ntinternals.net/UserMode/Undocumented%20Functions/System%20I
nformation/Structures/SYSTEM_PROCESS_INFORMATION.html

The peculiarity of NtQuerySystemInformation() is that it succeeds for basically 
ALL processes except for PID 0!
On the other hand it is sensibly slower compared to using native APIs.
As such, a natural approach seems to be using the current, native APIs 
implementation and fall back on using NtQuerySystemInformation in case of 
permission error. Something like:

def get_process_cpu_times():
    try:
        return _psutil_mswindows.get_process_cpu_times()    # native API method
    except AccessDenied:
        return _psutil_mswindows.get_process_cpu_times_2()  # alternative method

Obviously, a robust set of unit tests will have to be written in order to make 
sure that the two methods return the exact same value.
Follows a summary of Process methods which can apparently take benefit from 
this approach:

 ---------------------------------------------------------------------------
| psutil method          | Win API              | PINFO struct              |
 ---------------------------------------------------------------------------
| create_time           | GetProcessTimes       | CreateTime                |
| nice/priority         | GetPriorityClass      | BasePriority              |
| get_cpu_times()       | GetProcessTimes       | UserTime, KernelTime      |
| get_cpu_percent()     | GetProcessTimes       | UserTime, KernelTime      |
| get_num_handles()     | GetProcessHandleCount | HandleCount               |
| get_memory_info()     | GetProcessMemoryInfo  | WorkingSetSize and others |
| get_ext_memory_info() | GetProcessMemoryInfo  | WorkingSetSize and others |
| get_memory_percent()  | GetProcessMemoryInfo  | WorkingSetSize and others |
| get_threads()         | GetThreadTimes        | Threads*                  |
| get_io_counters()     | GetProcessIoCounters  | *OperationCount           |
 ---------------------------------------------------------------------------

@wj32.64, given the massive amount of code rewriting involved, I'd like to hear 
your opinion about this. Does it sounds reasonable and reliable to you?

Original issue reported on code.google.com by g.rodola on 13 Jul 2012 at 11:31

GoogleCodeExporter commented 8 years ago
The other way to handle this that comes to mind is similar to what we 
originally did with the first incarnation of psutil; since multiple pieces of 
information are retrieved with the same query, populate all those parts of the 
process information structure simultaneously, and potentially also cache the 
values when possible. Something like the deproxy lazy initialization we were 
using in early versions of psutil.

I realize caching might not be feasible if we're looking at volatile 
information like memory or CPU, so that would require some thinking. But if we 
can fetch multiple pieces of data in one call to NtQuerySystemInformation() 
then we're saving a lot of overhead in cases where someone is enumerating 
multiple process properties.

My main question/issue with using NtQuerySystemInformation() so heavily is 
reliability. Undocumented APIs can change at any time and may not be consistent 
from release to release. I'd be concerned that we might run into 
incompatibilities in structures across versions, but I don't know if that's the 
case. If it's remained consistent across at least the last few Windows releases 
then it's probably not a major concern.

Original comment by jlo...@gmail.com on 13 Jul 2012 at 12:43

GoogleCodeExporter commented 8 years ago
I don't think you have to worry about NtQuerySystemInformation being 
"undocumented". The chance of it changing is pretty low, since there are a lot 
of developers who use it. To address your concerns about performance - you have 
to choose between these two:

* NtQuerySystemInformation - a bit slower and requires memory allocation, but 
bypasses permissions
* Normal APIs (which all use NtQueryInformationProcess) - a bit faster, but 
requires a handle to the process

I like jloden's idea of caching properties for multiple processes, but of 
course you have to decide when to update your cached data. That could be a bit 
of a problem.

Original comment by wj32...@gmail.com on 13 Jul 2012 at 12:53

GoogleCodeExporter commented 8 years ago
Since we're talking about volatile info I can't see any consistent way to cache 
it except introducing a brand new CachedProcess class providing a refresh() 
method. Please note that we now have an as_dict() method though, which can be 
used to implement caching pretty flexibly by hand, as in:

>>> p = psutil.Process(pid)
>>> p._info = p.as_dict()   # save all current process info
>>> p._info['cpu_percent']  # access cached info

...later on:

>>> p._info = p.as_dict()   # update() cache

Also, note that I've already introduced caching where possible in latest 
release (ppid, name, exe, cmdline and create_time, process_iter() - issue 281 
and issue 301).

As for populating the struct simultaneously please note that using 
NtQuerySystemInformation() is *a lot* slower than using documented APIs (about 
-6x) so I'm not sure how much grouping would help.

Original comment by g.rodola on 13 Jul 2012 at 2:26

GoogleCodeExporter commented 8 years ago
Ok, this is now fixed.
The Process methods affected by this change are:

- create_time            r1449
- get_cpu_times()        r1448
- get_cpu_percent()      r1448 
- get_memory_info()      r1452
- get_memory_percent()   r1452
- get_num_handles()      r1450
- get_io_counters()      r1451

Note that we're now able to determine meaningful info even for PID 4, which 
return value was historically hard-coded in the python layer.

Original comment by g.rodola on 13 Jul 2012 at 8:25

GoogleCodeExporter commented 8 years ago
Fixed in version 0.6.0, released just now.

Original comment by g.rodola on 13 Aug 2012 at 4:25

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Updated csets after the SVN -> Mercurial migration:
r1448 == revision 5c5b03f3643f
r1449 == revision 2dcc29d215a7
r1450 == revision 6960659dac04
r1451 == revision 48a6b054f238
r1452 == revision 85677a2caf85

Original comment by g.rodola on 2 Mar 2013 at 12:11