nikademus79 / psutil

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

Process.__eq__ method causes problems if Process instances are stored in a list #211

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
run the following script:

import psutil, time

while 1:
    procs = [p for p in psutil.process_iter()]
    for p in procs:
        try:
            p.get_memory_info()
        except psutil.Error:
            procs.remove(p)
    time.sleep(1)
    for p in procs:
        try:
            p.get_memory_info()
        except psutil.Error:
            procs.remove(p)
    time.sleep(1)

...then try to open and kill a new process within the time.sleep() window (1 
second).

What is the expected output? What do you see instead?

The script will fail with:

Traceback (most recent call last):
  File "foo.py", line 43, in <module>
    procs.remove(p)
  File "/home/giampaolo/svn/psutil/psutil/__init__.py", line 132, in __eq__
    h2 = (other.pid, other.create_time)
  File "/home/giampaolo/svn/psutil/psutil/__init__.py", line 254, in create_time
    return self._platform_impl.get_process_create_time()
  File "/home/giampaolo/svn/psutil/psutil/_pslinux.py", line 332, in wrapper
    raise NoSuchProcess(self.pid, self._process_name)
psutil.error.NoSuchProcess: process no longer exists (pid=5442)

Please use labels and text to provide additional information.
What happens is procs.remove(p) call iterates over the list to figure out what 
the right list element to remove is.
In doing so it calls Process.__eq__ method (which we overridden) against the 
list elements until the right element is found.

We originally overridden __eq__ to implement is_running() method.
The logic behind is_running() is to test for equality with another Process 
object based on pid and creation time.
That is correct but at the time we did that we didn't consider that __eq__ 
method can be used in other cirumstances such as [].remove() which for no 
reason should raise a psutil-related exception.

The proposed fix is to move that logic from __eq__ to is_running() and leave 
__eq__ alone.

Original issue reported on code.google.com by g.rodola on 23 Sep 2011 at 2:37

GoogleCodeExporter commented 8 years ago
Fixed in r1137.

Original comment by g.rodola on 23 Sep 2011 at 3:05

GoogleCodeExporter commented 8 years ago
The main reason we overrode the __eq__  wasn't for is_running, it was so that 
we had the ability to compare Process objects directly. i.e. a user could do 
something like this:

pid1 = read_pid_from_file("foo")
pid2 = read_pid_from_file("bar")
p1 = psutil.Process(pid1)
p2 = psutil.Process(pid2)

if p1 == p2:
    # do something

It's not a huge loss of functionality, but that was why we originally did it 
that way. is_running was just implemented the way it was because we had just 
finished implementing the __eq__ method and it seemed logical to make use of 
it. 

Original comment by jlo...@gmail.com on 23 Sep 2011 at 3:27

GoogleCodeExporter commented 8 years ago

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

GoogleCodeExporter commented 8 years ago

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

GoogleCodeExporter commented 8 years ago

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

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Updated csets after the SVN -> Mercurial migration:
r1137 == revision dc6fa35bf17d

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