Issue1692335
Created on 2007-04-01 13:46 by zseil, last changed 2009-05-15 02:22 by ajaksu2.
|
msg52348 - (view) |
Author: Ziga Seilnacht (zseil) |
Date: 2007-04-01 13:46 |
|
Pickling exceptions fails when an Exception class
requires an argument in the constructor, but doesn't
call its base class' constructor. See this mail
for details:
http://mail.python.org/pipermail/python-dev/2007-April/072416.html
This patch simply moves initial args assignment to
BaseException.__new__. This should fix most of the
problems, because it is very unlikely that an
exception overwrites the __new__ method; exceptions
used to be old style classes, which don't support
the __new__ special method.
The args attribute is still overwritten in all the
__init__ methods, so there shouldn't be any backward
compatibility problems.
|
|
msg52349 - (view) |
Author: Ziga Seilnacht (zseil) |
Date: 2007-04-01 13:47 |
|
File Added: exc_args_25.diff
|
|
msg52350 - (view) |
Author: Ziga Seilnacht (zseil) |
Date: 2007-04-01 21:50 |
|
I'm attaching a test that Eric Huss sent in private
mail.
The test fails, because old exception pickles don't
have args for the reconstructor, but their __init__()
gets called anyway because they are new style classes
now.
The problem is in cPickle.InstanceNew() and
pickle.Unpickler._instantiate(). Those methods behave
differently depending on the type of the object
instantiated; they avoid the __init__() call when type
is an old style class.
There is nothing that can be done in the exception
classes to fix this issue; the fix would need to be
in the pickle and cPickle module.
File Added: test_exception_pickle.py
|
|
msg52351 - (view) |
Author: Eric Huss (ehuss) |
Date: 2007-06-15 00:34 |
|
I have stumbled across another scenario where unpickling fails. If your exception takes arguments, but you call Exception.__init__ with a different number of arguments, it will fail. As in:
class D(Exception):
def __init__(self, foo):
self.foo = foo
Exception.__init__(self)
|
|
msg52352 - (view) |
Author: Eric Huss (ehuss) |
Date: 2007-06-27 18:45 |
|
Added patch #1744398 as an alternate solution.
|
|
msg52353 - (view) |
Author: Georg Brandl (georg.brandl) |
Date: 2007-08-11 09:06 |
|
Attaching a new patch that fixes the
class D(Exception):
def __init__(self, foo):
self.foo = foo
Exception.__init__(self)
scenario by keeping the original args to __new__ as an exception attribute.
File Added: exception-pickling.diff
|
|
msg52355 - (view) |
Author: Neal Norwitz (nnorwitz) |
Date: 2007-08-11 18:27 |
|
self->args needs to be initialized to NULL because if the alloc of args failed, self->args would be uninitialized and deallocated. (I realize the alloc of an empty tuple will realistically never fail currently.)
I'm not sure if this could cause any new problems because of the behavior change, but the code itself looked fine to me. Hopefully someone with more knowledge of exceptions could take a look at the idea.
|
|
msg52356 - (view) |
Author: Eric Huss (ehuss) |
Date: 2007-08-12 01:33 |
|
Georg's new patch looks good to me, it seems to pass all the scenarios that I know of. You can close out my patch if you check this in. Thanks for looking into this!
|
|
msg52357 - (view) |
Author: Georg Brandl (georg.brandl) |
Date: 2007-08-12 08:00 |
|
The only question left is what to do if something reassigns exceptioninstance.args.
|
|
msg52359 - (view) |
Author: Ziga Seilnacht (zseil) |
Date: 2007-08-12 11:23 |
|
There is also the problem that Jim Fulton mentioned
in bug #1742889; if the user modifies the 'message'
attribute of an exception (or any other attribute
that is not stored in the __dict__), it won't get
serialized on pickling.
Attaching a new patch that fixes this by using the
__getstate__() pickling hook. It uses tp_members
and tp_getset members of the type object to find
all the attributes that need to be serialized.
The 2.5 patch also contains a fix for migrating
old exception pickles to Python 2.5. For that
I had to change cPickle and pickle. It would
be nice if Jim could comment if this change is
needed, since ZODB is probably the biggest user
of these modules.
The downside is that this patch is fairly big.
File Added: exception_pickling_25.diff
|
|
msg52360 - (view) |
Author: Ziga Seilnacht (zseil) |
Date: 2007-08-12 11:24 |
|
File Added: exception_pickling_26.diff
|
|
msg52361 - (view) |
Author: Georg Brandl (georg.brandl) |
Date: 2007-08-12 12:13 |
|
I wouldn't care too much about .message; it was new in 2.5 and is already deprecated in 2.6.
|
|
msg52362 - (view) |
Author: Ziga Seilnacht (zseil) |
Date: 2007-08-12 12:34 |
|
It's not just the message attribute; the problem is
that the current code expects that the exception
won't be modified between creation and unpickling.
For example:
>>> import pickle
>>> e = EnvironmentError()
>>> e.filename = "x"
>>> new = pickle.dumps(pickle.loads(e))
>>> print new.filename
None
>>> e = SyntaxError()
>>> e.lineno = 10
>>> new = pickle.loads(pickle.dumps(e))
>>> print new.lineno
None
|
|
msg52363 - (view) |
Author: Georg Brandl (georg.brandl) |
Date: 2007-08-12 13:24 |
|
Yes, this is what I meant with my last comment.
|
|
msg55165 - (view) |
Author: Georg Brandl (georg.brandl) |
Date: 2007-08-23 17:29 |
|
Raising priority.
|
|
msg55958 - (view) |
Author: Sean Reifschneider (jafo) |
Date: 2007-09-17 10:01 |
|
It's not clear to me what the next step is here, does one or more of the
other folks need to provide some input? Georg: If you are looking for
something from someone, can you assign the ticket to them?
|
|
msg56054 - (view) |
Author: Georg Brandl (georg.brandl) |
Date: 2007-09-20 08:47 |
|
IMO zseil's latest patch is the best one, but I need any other developer
to review and approve it.
|
|
msg60067 - (view) |
Author: Jaroslav Pachola (jarpa) |
Date: 2008-01-17 22:13 |
|
While zseil's patch for Python 2.5 works for me (on the current 2.5.1
download), svn version of Python 2.6 rejects the 2.6 patch. Attaching
fixed 2.6 patch (2 rejects, 1 fuzz fixed, patch works without complains
for me). I would be very glad if someone could review the patches and
maybe commit them.
|
|
msg60163 - (view) |
Author: Facundo Batista (facundobatista) |
Date: 2008-01-19 14:03 |
|
The last patch (exception_pickling_26.diff) applies cleanly, and all the
tests run ok.
System:
Python 2.6a0 (trunk:60073M, Jan 19 2008, 11:41:33)
[GCC 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)] on linux2
|
|
msg61967 - (view) |
Author: Guido van Rossum (gvanrossum) |
Date: 2008-02-01 17:38 |
|
I'm against including this in 2.5.2. It reeks of a new feature, and the
code looks like it would risk breaking other apps.
(I'm fine with adding this to 2.6 of course.)
|
|
msg61997 - (view) |
Author: Jaroslav Pachola (jarpa) |
Date: 2008-02-02 08:07 |
|
To me it seems more like a bug fix - in Python 2.4 and older the
pickling works well and the current 2.5 behavior really breaks existing
code. That's why I plead for inclusion in 2.5.2; because this issue
prevents our company to move to 2.5.
|
|
msg61999 - (view) |
Author: Guido van Rossum (gvanrossum) |
Date: 2008-02-02 11:36 |
|
Understood. Can you get other developers on python-dev to weigh in?
Maybe I am over-estimating the danger.
|
|
msg62025 - (view) |
Author: Martin v. Löwis (loewis) |
Date: 2008-02-03 19:04 |
|
I tested exception_pickling_25.diff, and it may break existing code.
In 2.5.1, Exception("Hello,4).__reduce__() gives
(<type 'exceptions.Exception'>, ('Hello', 4))
With the patch, it gives
TypeError: can't pickle Exception objects
IMO, that is an unacceptable change for a bugfix release.
Aside: please give unique file names to the patches, or remove patches
if you want to replace a previous patch.
|
|
msg70453 - (view) |
Author: Benjamin Peterson (benjamin.peterson) |
Date: 2008-07-31 01:51 |
|
How is this coming? Can we apply this to 2.6?
|
|
msg75955 - (view) |
Author: Ziga Seilnacht (zseil) |
Date: 2008-11-17 11:37 |
|
Sorry for the long silence. I think that this patch is not relevant
anymore. The code that uses exception pickling already had to be adapted
to changes in Python 2.5, so there is no need to change the pickling
again and risk breaking user code. I'll close it in a few days if nobody
objects.
|
|
msg76020 - (view) |
Author: Eric Huss (ehuss) |
Date: 2008-11-18 22:06 |
|
I'm disappointed to see this closed. Exception pickling is still broken
in some cases in 2.6.
|
|
msg76026 - (view) |
Author: Guido van Rossum (gvanrossum) |
Date: 2008-11-18 22:52 |
|
OK, reopening. Can you post an example that fails today?
|
|
msg76028 - (view) |
Author: Eric Huss (ehuss) |
Date: 2008-11-18 23:07 |
|
In the attached test_exception_pickle.py file, class C and D cannot be
unpickled (raises TypeError).
class C(Exception):
"""Extension with values, args not set."""
def __init__(self, foo):
self.foo = foo
class D(Exception):
"""Extension with values, init called with no args."""
def __init__(self, foo):
self.foo = foo
Exception.__init__(self)
There are also some other exceptions that fail to be handled properly.
See the exception_pickling_26.diff for some unittests. In particular, I
see SlotedNaiveException failing.
|
|
| Date |
User |
Action |
Args |
| 2009-05-15 02:22:51 | ajaksu2 | set | stage: patch review type: behavior versions:
+ Python 3.1, - Python 2.5 |
| 2008-11-18 23:07:21 | ehuss | set | messages:
+ msg76028 |
| 2008-11-18 22:54:39 | benjamin.peterson | set | nosy:
- benjamin.peterson |
| 2008-11-18 22:52:19 | gvanrossum | set | status: closed -> open resolution: out of date -> messages:
+ msg76026 |
| 2008-11-18 22:06:07 | ehuss | set | messages:
+ msg76020 |
| 2008-11-17 17:46:34 | gvanrossum | set | status: open -> closed resolution: out of date |
| 2008-11-17 11:37:01 | zseil | set | messages:
+ msg75955 |
| 2008-07-31 01:51:23 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages:
+ msg70453 |
| 2008-02-04 19:44:49 | alexandre.vassalotti | set | nosy:
+ alexandre.vassalotti |
| 2008-02-03 19:04:04 | loewis | set | nosy:
+ loewis messages:
+ msg62025 |
| 2008-02-02 11:36:55 | gvanrossum | set | messages:
+ msg61999 |
| 2008-02-02 08:07:22 | jarpa | set | messages:
+ msg61997 |
| 2008-02-01 17:38:02 | gvanrossum | set | nosy:
+ gvanrossum messages:
+ msg61967 |
| 2008-01-19 14:03:52 | facundobatista | set | nosy:
+ facundobatista messages:
+ msg60163 |
| 2008-01-17 22:13:52 | jarpa | set | files:
+ exception_pickling_26.diff nosy:
+ jarpa messages:
+ msg60067 |
| 2007-11-20 02:00:03 | brett.cannon | set | nosy:
+ brett.cannon |
| 2007-09-20 08:47:33 | georg.brandl | set | messages:
+ msg56054 |
| 2007-09-17 10:01:29 | jafo | set | assignee: georg.brandl messages:
+ msg55958 nosy:
+ jafo |
| 2007-08-28 08:40:38 | jafo | set | messages:
- msg52358 |
| 2007-08-28 08:40:18 | jafo | set | messages:
- msg52354 |
| 2007-08-23 17:29:54 | georg.brandl | set | priority: normal -> critical versions:
+ Python 2.6 messages:
+ msg55165 severity: normal -> major title: Move initial args assignment to BaseException.__new__ -> Fix exception pickling: Move initial args assignment to BaseException.__new__ |
| 2007-08-23 17:28:00 | georg.brandl | link | issue1744398 superseder |
| 2007-08-23 17:24:47 | georg.brandl | link | issue1742889 superseder |
| 2007-04-01 13:46:15 | zseil | create | |
|