Dumping a recursion error causes a recursion error

[ml31415]

Consider the following code sample:

import sys
from cPickle import dumps
from tblib.pickling_support import pickle_traceback
sys.setrecursionlimit(50)

def f(x):
    return f(x)

try:
    f(1)
except RuntimeError:
    info = sys.exc_info()
    pickle_traceback(info[2])

Instead of properly storing the traceback up to the recursion limit, the exception handling raises a recursion error itself:

Traceback (most recent call last):
  File "crashdemo.py", line 13, in <module>
    pickle_traceback(info[2])
  File "/usr/local/lib/python2.7/dist-packages/tblib/pickling_support.py", line 20, in pickle_traceback
    return unpickle_traceback, (Frame(tb.tb_frame), tb.tb_lineno, tb.tb_next and Traceback(tb.tb_next))
  File "/usr/local/lib/python2.7/dist-packages/tblib/__init__.py", line 63, in __init__
    self.tb_next = Traceback(tb.tb_next)
...........
  File "/usr/local/lib/python2.7/dist-packages/tblib/__init__.py", line 57, in __init__
    self.tb_frame = Frame(tb.tb_frame)
  File "/usr/local/lib/python2.7/dist-packages/tblib/__init__.py", line 50, in __init__
    if k in ("__file__", "__name__")
RuntimeError: maximum recursion depth exceeded in cmp

This issue initially occured with gevent, as tblib is vendored there: gevent/gevent#954

[jamadden]

I'm happy to help in whatever capacity. I can put together a PR if we can agree on an approach.

[ionelmc]

I guess we could replace the recursive Traceback(tb.next) with something that uses a stack (using collections.deque maybe?).

One cheap way to do it is to just implement this in pickle_traceback, however I would like to also make Traceback.__init__ immune to this problem. Perhaps adding a no_recurse=False argument to Traceback.__init__ could make this possible (the __init__ will add no_recurse=True and patch the other objects with the right tb_next). Interested in doing something like this? Other ideas?

[ionelmc]

Errrm... forgot about the double negative rule. recurse=True is better.

[jamadden]

This was my tentative approach:

class Traceback(object):
   ...
    @classmethod
    def from_tb(cls, tb):
        tbs = []
        while tb is not None:
            traceback = object.__new__(cls)
            traceback.tb_frame = Frame(tb.tb_frame)
            traceback.tb_lineno = int(tb.tb_lineno)
            traceback.tb_next = None
            if tbs:
                tbs[-1].tb_next = traceback
            tbs.append(traceback)
            tb = tb.tb_next
  ...

def pickle_traceback(tb):
    return unpickle_traceback, (Frame(tb.tb_frame), tb.tb_lineno, 
                                                   tb.tb_next and Traceback.from_tb(tb.tb_next))

I suppose the classmethod could be integrated into __init__ and then switched on and off with an argument, but no caller would ever pass that argument, Traceback would only pass it to itself, which just seems weird.

In fact, if it weren't for BWC, I might like to go one further and change the __init__ argument to match the signature of unpickle_traceback (thus simplifying both this classmethod and unpickle_traceback) and update any other callers of Traceback(tb) to use Traceback.from_tb (hmm, that could simplify pickle_traceback too, by automatically handling the missing tb.tb_next).

[ionelmc]

Actually, the from_tb code could be integrated in __init__ - no special recurse=false argument would be necessary if __init__ can make all the subsequent instances using __new__ right?

[ionelmc]

Ooops, mistyped a name above, edited now - should make more sense.

[jamadden]

Yes, that can work too.

[jamadden]

So that lets us create and pickle a Traceback, but we can't call as_traceback() on it, because that also uses recursion. Perhaps a similar approach can work there. Checking...