Popen.__del__() crashes with RuntimeError('maximum recursion depth exceeded while calling a Python object')

[dimp-gh]

Hi,

First of all, thanks for maintaining psutil. It's a wonderful cross-platform library and I'm pretty sure that every Python programmer used it at least once, if not daily.

Back to the point. It looks like bdcde7e introduced a regression which causes nasty 'maximum recursion depth exceeded' crashes on both Linux and Windows.

As crashing code seems to be in Popen.__del__(), I'm having trouble tracing it or creating a simple script to reproduce crash, but hopefully i would be able to craft one later today or tomorrow.

I'm pretty sure the issue is in Popen.__del__(), though, because I tried to inherit a CustomPopen class from psutil.Popen, created a dummy __del__() implementation (that doesn't call parent's __del__()) — and my program doesn't crash anymore. Also, if I downgrade psutil to 4.3.1 — there are no crashes too.

[giampaolo]

Hi, thanks and I'm glad psutil is useful to you. Do you have a stack trace? I understand you can't reproduce the issue but the initial part of the traceback showing what call caused the recursion error would be nice to see.

[dimp-gh]

It looks like it's an internal Python crash, as all I'm getting is the wall of the following messages:

Exception RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored
Exception RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored
Exception RuntimeError: RuntimeError('maximum recursion depth exceeded while calling a Python object',) in  ignored

So there's no stacktrace at all.

[giampaolo]

Mm weird... can you paste the script you're using (or part of it)?

[dimp-gh]

Okay, i was finally able to craft a script.

import gc
from time import sleep

from psutil import Popen


# NOTE: there's no binary with name `1` in my $PATH so OSError is expected
def reproduce1():
    proc = None
    try:
        proc = Popen(['1'])
    except OSError:
        pass
    proc.__del__()


def reproduce2():
    try:
        Popen(['1'])
    except OSError:
        pass

    # a few iterations are needed for GC to recognize that Popen instance has no references
    for _ in range(3):
        sleep(1)
        gc.collect()  # GC will call Popen.__del__ when collecting it


if __name__ == '__main__':
    reproduce1()

Both reproduce1() and reproduce2() cause the crash on Python 2.7.6. It looks like with Python 3.4.3 everything works fine, though.

[giampaolo]

Wow! That is weird! It looks like a Python bug. I also verified the issue goes way if I remove __del__ from Popen class but I really don't understand why.

[dimp-gh]

It looks like Python has fancy semantics for exceptions raised from __del__: https://docs.python.org/2/reference/datamodel.html#object.__del__

Normally, exceptions from __del__ are just ignored, but maybe stack overflow is the kind of exception that can't be simply ignored.

But yeah, it's starting to look like Python bug to me, too.

[dimp-gh]

Little side note: I'm not sure it's necessary to explicitly call self.__subproc.__del__(*args, **kwargs) in Popen.__del__(). I mean, GC will sooner or later collect __subproc anyway when no references to it are left. Assuming the only reference to __subproc is in Popen instance itself, it should be collected automatically pretty soon.

[giampaolo]

I'm also not sure about the introduction of __del__. I will try to investigate this a little bit further but it's likely I'm gonna release a new version without __del__ pretty soon as it looks like the most obvious solution.

[giampaolo]

One more question: how did you bump into this? Did you were explicitly calling __del__? Because you're not supposed to.

[dimp-gh]

Yesterday our CI stack for Taurus (TravisCI and Appveyor) started failing with this exact problem (1, 2). I quickly examined it and got to a conclusion that the new psutil release was the main source of the issue.

Then i started fiddling around with the test suite locally and the only two ways to reliably reproduce the issue i found were:

1. Invoke __del__ on Popen directly to cause the crash (function reproduce1 in the above code sample).
2. Force-run gc.collect() a few times to cause garbage collector to collect free Popen instance and invoke its __del__, therefore causing the crash (function reproduce2 in the above code sample).

And yeah, I can assure you that there are no explicit calls to Popen.__del__ in our project.

[giampaolo]

OK, we definitively need to release a new version then. Will do it soon, hopefully today.
Thanks for your support.

[giampaolo]

OK, a new version is out.

[dimp-gh]

Yep, can verify. Our test suite passes all tests with 4.4.1.

Thanks a lot for the quick response and the fix.