classification
Title: pickling of large recursive structures crashes cPickle
Type: crash Stage:
Components: Library (Lib) Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: alexandre.vassalotti, amaury.forgeotdarc, bkline, cyhawk, esrever_otua, facundobatista, jcea, loewis, schmir
Priority: normal Keywords: patch

Created on 2008-04-27 07:53 by cyhawk, last changed 2008-07-16 16:35 by facundobatista. This issue is now closed.

Files
File name Uploaded Description Edit
bugdemo.py cyhawk, 2008-04-27 07:53 causes silent termination ("ok" is not printed) on my computer
test_cpickle.diff facundobatista, 2008-06-22 22:29 Patch to the tests, to raise this issue
cpickle.patch amaury.forgeotdarc, 2008-06-25 21:41
cpickle2.patch amaury.forgeotdarc, 2008-06-25 21:53 use Py_EnterRecursiveCall instead of self->nesting
Messages (15)
msg65876 - (view) Author: Daniel Darabos (cyhawk) Date: 2008-04-27 07:53
The documentation[1] says:

  Trying to pickle a highly recursive data structure may exceed the
  maximum recursion depth, a RuntimeError will be raised in this
  case. You can carefully raise this limit with sys.setrecursionlimit().

The lightweight pickle module handles this problem correctly (in that it
raises a RuntimeError), but cPickle sometimes raises KeyError instead,
or just silently terminates the interpreter (=crashes). (I have not been
able to pinpoint what it depends on. In the attached example I get
silent termination, but if instead of lists I use sets to describe the
connections, I get the RuntimeError.)

This was mentioned in issue 2480, but that has now been changed to a
feature request to eliminate recursion altogether. That may have a lower
priority, but this crash can be hard to diagnose in a complex
application, and I am not sure if sys.setrecursionlimit() affects
cPickle behavior (I guess not).

[1]: http://docs.python.org/lib/node317.html
msg65878 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-04-27 08:23
What operating system and compiler are you using?
msg65879 - (view) Author: Daniel Darabos (cyhawk) Date: 2008-04-27 08:45
Python 2.5 (r25:51908, Sep 19 2006, 09:52:17) [MSC v.1310 32 bit
(Intel)] on win32

(Windows XP Professional 32 bits)
msg68587 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-06-22 22:29
Could you please tell me if this problem arises with this test?
msg68588 - (view) Author: Daniel Darabos (cyhawk) Date: 2008-06-22 22:35
I have just quickly pasted it into an interpreter.

Python 2.5 (r25:51908, Sep 19 2006, 09:52:17) [MSC v.1310 32 bit
(Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> class Node(object):
...     pass
...
>>> nodes = [Node() for i in range(500)]
>>> for n in nodes:
...     n.connections = list(nodes)
...     n.connections.remove(n)
...
>>> import cPickle
>>> s = cPickle.dumps( n )

After this line, the interpreter terminated without any further output
(no Python exception and no Windows "General Exception" message box either).

Is it sufficient, or would you prefer me to run the test properly?
msg68589 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-06-22 22:38
Daniel, it'd be great, because it does not crash in linux, so I can not
test it... and I have a patch to apply (see issue 3165), so I wanted to
test it that way.
msg68591 - (view) Author: Daniel Darabos (cyhawk) Date: 2008-06-22 22:43
It works for me as a test case too:

test_deep_recursive (__main__.cPickleDeepRecursive) ... ERROR

======================================================================
ERROR: test_deep_recursive (__main__.cPickleDeepRecursive)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\devel\Python25\Lib\test\test_cpickle.py", line 107, in
test_deep_recursive
    self.assertRaises(RuntimeError, cPickle.dumps, n)
  File "C:\devel\Python25\lib\unittest.py", line 320, in failUnlessRaises
    callableObj(*args, **kwargs)
KeyError: 13352688
msg68601 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-06-23 01:04
Commited this patch to the test cases, and the patch from #3165 to fix
the problem, thank you all!
msg68745 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-06-25 19:25
I reverted the patch, because of #3179, so I'm reopening this.

Note that I left the test in the suite, but commented out (if you find a
patch that solves this, activate the test again).
msg68759 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-06-25 21:41
It seems that each self->nesting++ should be balanced with
self->nesting--. Attached patch enables all tests and corrects the issue
msg68760 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-06-25 21:53
Another version of the patch, that removes the self->nesting member and
uses the Py_EnterRecursiveCall machinery.
I prefer it because it takes into account the depth of the current
python call stack.
msg68764 - (view) Author: Ralf Schmitt (schmir) Date: 2008-06-26 06:36
The tests pass on my machine (64 bit debian testing) with cpickle2.patch.
msg68982 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-06-30 01:11
Great, Amaury, I applied your second patch... it make all tests pass ok, :)

Commited in 64595.

Thank you all!!
msg69761 - (view) Author: Darryl Dixon (esrever_otua) Date: 2008-07-16 04:12
Please check issue #3338 for a dup of this issue with a testcase that
continues to fail after the application of r64595
msg69799 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-07-16 16:35
Thanks Darryl.

We'll continue in that issue, as the patched commited in this one did
not introduce a regression (it just didn't fix the other bug also).
History
Date User Action Args
2008-07-16 16:35:27facundobatistasetmessages: + msg69799
2008-07-16 04:12:23esrever_otuasetnosy: + esrever_otua
messages: + msg69761
2008-06-30 01:11:29facundobatistasetstatus: open -> closed
resolution: fixed
messages: + msg68982
2008-06-26 06:36:30schmirsetmessages: + msg68764
2008-06-25 21:53:48amaury.forgeotdarcsetfiles: + cpickle2.patch
messages: + msg68760
2008-06-25 21:41:28amaury.forgeotdarcsetfiles: + cpickle.patch
nosy: + amaury.forgeotdarc
messages: + msg68759
2008-06-25 19:25:51facundobatistasetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg68745
2008-06-23 01:04:22facundobatistasetstatus: open -> closed
resolution: fixed
messages: + msg68601
2008-06-22 22:43:12cyhawksetmessages: + msg68591
2008-06-22 22:38:10facundobatistasetmessages: + msg68589
2008-06-22 22:35:54cyhawksetmessages: + msg68588
2008-06-22 22:29:42facundobatistasetfiles: + test_cpickle.diff
nosy: + facundobatista
messages: + msg68587
keywords: + patch
2008-04-27 08:45:16cyhawksetmessages: + msg65879
2008-04-27 08:23:30loewissetnosy: + loewis
messages: + msg65878
2008-04-27 07:53:08cyhawkcreate