[Patches] [ python-Patches-1692335 ] Move initial args assignment to BaseException.__new__

[SourceForge.net]

Patches item #1692335, was opened at 2007-04-01 06:46
Message generated for change (Comment added) made by ehuss
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1692335&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Ziga Seilnacht (zseil)
Assigned to: Nobody/Anonymous (nobody)
Summary: Move initial args assignment to BaseException.__new__

Initial Comment:
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.



----------------------------------------------------------------------

Comment By: Eric Huss (ehuss)
Date: 2007-08-11 18:33

Message:
Logged In: YES 
user_id=393416
Originator: NO

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!


----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2007-08-11 11:27

Message:
Logged In: YES 
user_id=33168
Originator: NO

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.

----------------------------------------------------------------------

Comment By: Georg Brandl (gbrandl)
Date: 2007-08-11 02:06

Message:
Logged In: YES 
user_id=849994
Originator: NO

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

----------------------------------------------------------------------

Comment By: Georg Brandl (gbrandl)
Date: 2007-08-11 02:06

Message:
Logged In: YES 
user_id=849994
Originator: NO

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

----------------------------------------------------------------------

Comment By: Eric Huss (ehuss)
Date: 2007-06-27 11:45

Message:
Logged In: YES 
user_id=393416
Originator: NO

Added patch #1744398 as an alternate solution.


----------------------------------------------------------------------

Comment By: Eric Huss (ehuss)
Date: 2007-06-14 17:34

Message:
Logged In: YES 
user_id=393416
Originator: NO

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)


----------------------------------------------------------------------

Comment By: Ziga Seilnacht (zseil)
Date: 2007-04-01 14:50

Message:
Logged In: YES 
user_id=1326842
Originator: YES

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

----------------------------------------------------------------------

Comment By: Ziga Seilnacht (zseil)
Date: 2007-04-01 06:47

Message:
Logged In: YES 
user_id=1326842
Originator: YES

File Added: exc_args_25.diff

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1692335&group_id=5470