peter-hst / psutil

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

Module init fails on missing "Buffers" line in /proc/meminfo #218

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Load the module on a system that doesn't have a "Buffers" line in 
/proc/meminfo

What is the expected output? What do you see instead?
The module doesn't load:
File "psutil/__init__.py", line 84, in <module>
  File "psutil/_pslinux.py", line 122, in phymem_usage
TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'

What version of psutil are you using? What Python version?
0.3.0, Python 2.5

On what operating system? Is it 32bit or 64bit version?
Debian 6.0, 64bit

Please provide any additional information below.

The system is a virtual machine, cat /proc/meminfo is missing the "Buffers" 
line which causes the module init to fail. I propose defaulting to zero for all 
values and not throwing exceptions when a line is missing.

cat /proc/meminfo:
MemTotal:        1048576 kB
MemFree:          865284 kB
Cached:           107968 kB
Active:           132580 kB
Inactive:          24664 kB
Active(anon):      49208 kB
Inactive(anon):       68 kB
Active(file):      83372 kB
Inactive(file):    24596 kB
Unevictable:           0 kB
Mlocked:               0 kB
SwapTotal:             0 kB
SwapFree:              0 kB
Dirty:                 0 kB
AnonPages:         49276 kB
Shmem:              2632 kB
Slab:              14632 kB
SReclaimable:       9220 kB
SUnreclaim:         5412 kB

free:
             total       used       free     shared    buffers     cached
Mem:       1048576     183296     865280          0          0     107968
-/+ buffers/cache:      75328     973248
Swap:            0          0          0

Original issue reported on code.google.com by thez...@gmail.com on 7 Oct 2011 at 1:26

GoogleCodeExporter commented 9 years ago
Actually, I think two things need to be done:
1. Never raise an exception for these functions (or only raise documented and 
psutil specific exceptions)
2. Don't run any of that code in the __init__.py file, instead just bind 
function names appropriately

Let me know if you want me to help with any of this or if you disagree.

Original comment by thez...@gmail.com on 7 Oct 2011 at 5:48

GoogleCodeExporter commented 9 years ago
Hi, sorry for the delay in responding.

> 1. Never raise an exception for these functions (or only raise
> documented and psutil specific exceptions)

This error represents a bug in psutil therefore raising a non-psutil exception 
makese perfect sense to me.
We do the same in the underlying C code: when a certain C call fails with an 
unexpected error we raise RuntimeError, which most of the times represents a 
something which should be fixed (and signaled here).

> 2. Don't run any of that code in the __init__.py file, 
> instead just bind function names appropriately

How would you do that exactly?
NUM_CPUS, BOOT_TIME and TOTAL_PHYMEM are all global constants and as such we 
are forced to set them at import time.
Note that I don't like that either, but I don't see how to do that otherwise.
If you can't propose a solution in this sense it is fine with me to default 
"Buffers" to zero.

Original comment by g.rodola on 13 Oct 2011 at 6:11

GoogleCodeExporter commented 9 years ago
Out of curiosity, it is strange that /proc/meminfo doesn't provide Buffers.
Could you please paste the output of "free" command?

Original comment by g.rodola on 13 Oct 2011 at 6:13

GoogleCodeExporter commented 9 years ago
Ok, in r1166 we are now retrieving total, free and buffers memory info in C 
instead of parsing /proc/meminfo in python.
This should be more portable and fix your issue.
Can you confirm?

Original comment by g.rodola on 17 Oct 2011 at 10:22

GoogleCodeExporter commented 9 years ago
Thank you for your response.

> > 1. Never raise an exception for these functions (or only raise
> > documented and psutil specific exceptions)
>
> This error represents a bug in psutil therefore raising a non-psutil exception
> makese perfect sense to me.
> We do the same in the underlying C code: when a certain C call fails with an
> unexpected error we raise RuntimeError, which most of the times represents a
> something which should be fixed (and signaled here).

Yes, in this case it was to be expected because psutil didn't anticipate this 
line to be missing but there are other instances where psutil handles the case 
of a missing line with an (undocumented) "RuntimeError" exception.

> > 2. Don't run any of that code in the __init__.py file, 
> > instead just bind function names appropriately
>
> How would you do that exactly?
> NUM_CPUS, BOOT_TIME and TOTAL_PHYMEM are all global constants and as such we 
are
> forced to set them at import time
> Note that I don't like that either, but I don't see how to do that otherwise.
> If you can't propose a solution in this sense it is fine with me to default
> "Buffers" to zero.

I'd suggest getting these values from functions when first needed. The 
functions can cache their results in a static variable so there would be (close 
to) no performance impact.

> Out of curiosity, it is strange that /proc/meminfo doesn't provide Buffers.
> Could you please paste the output of "free" command?

Yes, strange indeed, I have never seen this before. The output of "free" is in 
the initial post.

Unfortunately I don't have access to this machine anymore and so far I haven't 
seen this line missing on any other installation but to me this sounds like a 
good replacement for the function in question.

Original comment by thez...@gmail.com on 18 Oct 2011 at 5:13

GoogleCodeExporter commented 9 years ago
> Yes, in this case it was to be expected because psutil didn't anticipate this 
line 
> to be missing but there are other instances where psutil handles the case of 
a 
> missing line with an (undocumented) "RuntimeError" exception.

Other cases are exactly the same: they represent a bug in psutil.
If we document this we should also document all other possible 
errors/exceptions we might have because of a bug inside psutil (TypeError for 
bad args passed to a function, OSError(errno) for an underlying C call which 
has failed, etc...), which obviously we can't do.

> I'd suggest getting these values from functions when first needed.

They are needed at import time because they are provided in form of *global 
constants* of psutil/__init__.py module and they are part of the public API:
http://code.google.com/p/psutil/source/browse/tags/release-0.3.0/psutil/__init__
.py#82
As such, we are forced to call those functions at import time, no matter what.
This was probably a bad design choice but NUM_CPUS, BOOT_TIME and TOTAL_PHYMEM 
are there since 0.1.0 version, so we can't get rid of them.
The best we can do is try to make them as robust as possible as we did now or 
as I did in r1165 with NUM_CPUS which is currently relying on 3 different 
methods for figuring out the number of cpus.
In case we *can't* follow this strategy we can decide to return a fake value 
(0) for those functions raising RuntimeError, but I'd like to see an issue 
filed first, if you know what I mean.

> Unfortunately I don't have access to this machine anymore

Urgh! :(
Well, according to its manual, sysinfo() is supposed to be there since kernel 
2.3.23, so let's assume this is fixed for now. =)
Thanks for signaling btw, this was a high priority one.

Original comment by g.rodola on 18 Oct 2011 at 6:10

GoogleCodeExporter commented 9 years ago

Original comment by g.rodola on 21 Oct 2011 at 11:44

GoogleCodeExporter commented 9 years ago

Original comment by g.rodola on 21 Oct 2011 at 11:45

GoogleCodeExporter commented 9 years ago

Original comment by g.rodola on 29 Oct 2011 at 3:44

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Updated csets after the SVN -> Mercurial migration:
r1165 == revision 37f787a2c3a8
r1166 == revision 729068fe5f84

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