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

[giampaolo]

From g.rodola on September 23, 2011 16:37:43

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: http://code.google.com/p/psutil/issues/detail?id=211

[giampaolo]

From g.rodola on September 23, 2011 08:05:02

Fixed in r1137 .

Status: FixedInSVN
Labels: Usability

[giampaolo]

From [email protected] on September 23, 2011 08:27:59

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.

[giampaolo]

From g.rodola on October 21, 2011 16:44:17

Labels: -Milestone-0.3.1

[giampaolo]

From g.rodola on October 21, 2011 16:45:27

Labels: Milestone-0.4.0

[giampaolo]

From g.rodola on October 28, 2011 20:44:14

Status: Fixed

[giampaolo]

From g.rodola on March 02, 2013 04:03:48

Updated csets after the SVN -> Mercurial migration: r1137 == revision 
dc6fa35bf17d