Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RecursionError after exception in the psutil.Popen.__init__() #1121

Open
Cykooz opened this issue Sep 2, 2017 · 24 comments
Open

RecursionError after exception in the psutil.Popen.__init__() #1121

Cykooz opened this issue Sep 2, 2017 · 24 comments
Labels

Comments

@Cykooz
Copy link

Cykooz commented Sep 2, 2017

Look at this code from constructor of psutil.Popen:

self.__subproc = subprocess.Popen(*args, **kwargs)

If it raises exception, then self.__subproc will be absent in the instance of psutil.Popen. In this case follow code will causes an exception RecursionError.

return object.__getattribute__(self.__subproc, name)

This bug can be fixed very simple - you will need to add a default value of __subproc at class level:

class Popen(Process):
    __subproc = None

    def __init__(self, *args, **kwargs):
        self.__subproc = subprocess.Popen(*args, **kwargs)
        self._init(self.__subproc.pid, _ignore_nsp=True)
@giampaolo
Copy link
Owner

How can I reproduce this?

@Cykooz
Copy link
Author

Cykooz commented Sep 3, 2017

from raven.base import DummyClient
import psutil

client = DummyClient()
client.is_enabled = lambda: True

try:
    proc = psutil.Popen('program_not_found')
except Exception as e:
    client.captureException()

@giampaolo
Copy link
Owner

Can't reproduce it. I get:

Traceback (most recent call last):
  File "foo.py", line 6, in <module>
    proc = psutil.Popen('program_not_found')
  File "/home/giampaolo/svn/psutil/psutil/__init__.py", line 1384, in __init__
    self.__subproc = subprocess.Popen(*args, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 711, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1343, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

@Cykooz
Copy link
Author

Cykooz commented Sep 7, 2017

I uses Python 3.6. Maybe the error only appears in Python 3.

@giampaolo
Copy link
Owner

I can't reproduce it. Please show me the full traceback. Actually can you do that (reproduce the issue) from the interactive python shell itself and paste the output?

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

@giampaolo
Copy link
Owner

There's something weird. Before RecursionError I see 'Popen' object has no attribute '_pid' but the traceback is truncated (perhaps due to client.captureException()?). Please just do:

>>> import psutil
>>> psutil.Popen('program_not_found')

Do you get an error like that?

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

The whole point is in using raven (it is a client for Sentry). Without it, the RecursionError error does not occur, because default traceback from Python does not look at the contents of objects inside the frame with an error.

@giampaolo
Copy link
Owner

So it does not occur if you remove client.captureException()?

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

Traceback is not truncated - it ends with RecursionError, because raven tries to get some information about the Popen instance (it calls method __repr__() for it), in which an exception occurs.

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

Yes, RecursionError does not occur if used default Python traceback.

@giampaolo
Copy link
Owner

If you define __subproc as a class attribute what error do you get? Can you see AttributeError: 'Popen' object has no attribute '_pid'? Actually I'm more interested in understanding where that comes from rather than RecursionError.

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

  • raven gets Popen instance as self variable from code frame of Popen.__init__() .
  • Next, raven calls something like str(variable). This leads to calling this code:

psutil/psutil/__init__.py

Lines 441 to 454 in fe0799f

def __str__(self):
try:
pid = self.pid
name = repr(self.name())
except ZombieProcess:
details = "(pid=%s (zombie))" % self.pid
except NoSuchProcess:
details = "(pid=%s (terminated))" % self.pid
except AccessDenied:
details = "(pid=%s)" % (self.pid)
else:
details = "(pid=%s, name=%s)" % (pid, name)
return "%s.%s%s" % (self.__class__.__module__,
self.__class__.__name__, details)

  • pid = self.pid leads to calling this code:

    psutil/psutil/__init__.py

    Lines 1411 to 1419 in fe0799f

    def __getattribute__(self, name):
    try:
    return object.__getattribute__(self, name)
    except AttributeError:
    try:
    return object.__getattribute__(self.__subproc, name)
    except AttributeError:
    raise AttributeError("%s instance has no attribute '%s'"
    % (self.__class__.__name__, name))
  • The first exception that will occur here is AttributeError: 'Popen' object has no attribute '_pid'
  • Then the code return object.__getattribute__(self.__subproc, name) will be called. This again causes the call to Popen.__getattribute__() but already in order to get the __subproc.

@giampaolo
Copy link
Owner

OK, good. What traceback do you get by defining __subproc as a class attribute?

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

https://gist.github.com/Cykooz/72ffb05e65112c06ea9b7e718d0d387f

As you can see, in this case python-shell did not fall and did not exit to the console.

@giampaolo
Copy link
Owner

giampaolo commented Sep 8, 2017

Mmmm... probably better but still incorrect as the right exception should be FileNotFoundError not AttributeError. I assume raven is doing something wrong here. I am tempted to assume it's a raven's bug, not psutil's.

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

Exception FileNotFoundError located at the start of the traceback. All other exception occur because raven calls return repr(value), where value is instance of Popen. These exceptions do not matter, raven handles them correctly. As a result, raven will not be able to get information about the Popen instance, but this is not critical, because the instance has no useful information.

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

This is not bug of raven. raven only calls repr(value), where value is instance of Popen. This results in the whole process of the python falling with the error RecursionError.

Your code expects that a Popen instance (or the Popen class itself) must contain the __subproc variable. But if you have an error in the constructor of Popen, then the instance is created without this variable. Therefore, further access to this instance results in a fatal error RecursionError.

@giampaolo
Copy link
Owner

I don't understand how can raven call repr against the Popen instance since the error occurs when you try to instantiate the instance (hence there's no real "instance" at that point). How did you figure out it's raven calling repr()? I see no repr reference in the traceback you pasted. Are you able to identify where in the raven code this is done?

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

In Python, instance is created by __new__ method of class, not by __init__ method. Therefore, instance already exists when exception occur in __init__ method.

You can see repr at the end of the last traceback.

Traceback (most recent call last):
  File "/home/cykooz/buildout-eggs/raven-6.1.0-py2.py3-none-any.ovo/raven/utils/serializer/manager.py", line 76, in transform
    return repr(value)
  File "/home/cykooz/buildout-eggs/psutil-5.2.2-py3.6-linux-x86_64.egg/psutil/__init__.py", line 446, in __repr__
    return "<%s at %s>" % (self.__str__(), id(self))
  File "/home/cykooz/buildout-eggs/psutil-5.2.2-py3.6-linux-x86_64.egg/psutil/__init__.py", line 432, in __str__
    pid = self.pid
  File "/home/cykooz/buildout-eggs/psutil-5.2.2-py3.6-linux-x86_64.egg/psutil/__init__.py", line 1405, in __getattribute__
    % (self.__class__.__name__, name))
AttributeError: Popen instance has no attribute 'pid'
'83a1e04e00654a99800c2e9b25f0eccc'

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

Ok, you may fix it by other way - by correct of getting self.__subproc in the Popen.__getattribute__().

    def __getattribute__(self, name):
        try:
            return object.__getattribute__(self, name)
        except AttributeError:
            try:
                subproc = object.__getattribute__(self, '__subproc')
                return object.__getattribute__(subproc, name)
            except AttributeError:
                raise AttributeError("%s instance has no attribute '%s'"
                                     % (self.__class__.__name__, name))

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

Or you may use __getattr__() instead of __getattribute__():

    def __getattr__(self, name):
        subproc = object.__getattribute__(self, '__subproc')
        return getattr(subproc, name)

@Cykooz
Copy link
Author

Cykooz commented Sep 8, 2017

Using access to the attributes of an instance through a dot (like self.__subproc) inside of __getattribute__() or __getattr__() methods is not a very good way.

@Cykooz
Copy link
Author

Cykooz commented Sep 14, 2017

Small fix for my last code example:

    def __getattr__(self, name):
        subproc = object.__getattribute__(self, '_Popen__subproc')
        return getattr(subproc, name)

An identifier of class-private member is textually replaced with _classname__varname (https://docs.python.org/3/tutorial/classes.html#private-variables)

@giampaolo giampaolo added the bug label Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants