[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Orivej Desh]

Change by Orivej Desh :


--
nosy: +orivej

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Guido van Rossum]

Guido van Rossum  added the comment:

Please don't discuss on closed issues. Open a new issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Ofer]

Ofer  added the comment:

@jason.coombs as far as I can tell, this only works in python3, but not in 
python2, where it still produces the same error.

--
versions: +Python 2.7 -Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Jason R. Coombs]

Jason R. Coombs  added the comment:

@slallum, That does seem to be a problem, though I do observe that the issue 
reported by bdb with CalledProcessError is no longer an issue:

>>> import subprocess, pickle
>>> try:
...   subprocess.check_call(['python', '-c', 'raise SystemExit(1)'])
... except Exception as e:
...   pickle.loads(pickle.dumps(e))
... 
CalledProcessError(1, ['python', '-c', 'raise SystemExit(1)'])

Looking into how CalledProcessError is defined 
(https://github.com/python/cpython/blob/79db11ce99332d62917be9d03b31494b1ff2f96a/Lib/subprocess.py#L60)
 may shed some light on the recommended way to make a pickleable Exception 
class that takes more than one argument.

Hmm. It seems it does it by not calling the superclass __init__. Indeed, 
following that model it seems to work:

import pickle
class MultipleArgumentsError(Exception):
def __init__(self, a, b):
self.a = a
self.b = b

err = pickle.loads(pickle.dumps(MultipleArgumentsError('a', 'b')))
assert err.a == 'a'
assert err.b == 'b'

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Ofer]

Ofer  added the comment:

Perhaps this is a problem for a different issue, but pickling custom exceptions 
fails when the exception gets more than one argument:

import pickle
class MultipleArgumentsError(Exception):
def __init__(self, a, b):
self.a = a
self.b = b
Exception.__init__(self, a)

pickle.loads(pickle.dumps(MultipleArgumentsError('a', 'b')))


this code produces the following error:

Traceback (most recent call last):
  File "/tmp/multiple_arguments_exception.py", line 8, in 
pickle.loads(pickle.dumps(MultipleArgumentsError('a', 'b')))
  File 
"/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/pickle.py",
 line 1388, in loads
return Unpickler(file).load()
  File 
"/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/pickle.py",
 line 864, in load
dispatch[key](self)
  File 
"/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/pickle.py",
 line 1139, in load_reduce
value = func(*args)
TypeError: __init__() takes exactly 3 arguments (2 given)

--
nosy: +slallum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Roundup Robot]

Roundup Robot added the comment:

New changeset 2c9f7ed28384 by R David Murray in branch '3.2':
#17296: backport fix for issue 1692335, naive exception pickling.
http://hg.python.org/cpython/rev/2c9f7ed28384

New changeset 67c27421b00b by R David Murray in branch '3.3':
Null merge for issue 1692335 backport.
http://hg.python.org/cpython/rev/67c27421b00b

New changeset 94f107752e83 by R David Murray in branch 'default':
Null merge for issue 1692335 backport.
http://hg.python.org/cpython/rev/94f107752e83

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Georg Brandl]

Georg Brandl added the comment:

Richard, can the issue be closed?

--
priority: release blocker - critical

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk added the comment:

 Richard, can the issue be closed?

I guess so (although the change could arguably be back ported).

Pickling of Exception classes is still somewhat dodgy because an example like

class Error(Exception):
def __init__(self, x):
Exception.__init__(self)
self.x = x

is still not picklable.

Any further improvements can be discussed in a separate issue, so I will close.

--
resolution:  - fixed
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Georg Brandl]

Georg Brandl added the comment:

Please reopen if you think it should be backported.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Georg Brandl]

Georg Brandl ge...@python.org added the comment:

OK, finally I think this can go in.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Roundup Robot]

Roundup Robot added the comment:

New changeset 68e2690a471d by Richard Oudkerk in branch 'default':
Issue #1692335: Move initial args assignment to BaseException.__new__
http://hg.python.org/cpython/rev/68e2690a471d

--
nosy: +python-dev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Łukasz Langa]

Changes by Łukasz Langa luk...@langa.pl:


--
assignee: lukasz.langa - sbt
stage: patch review - commit review
versions:  -Python 2.7, Python 3.2

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk shibt...@gmail.com added the comment:

Here is a minimal patch against default.

It is a clear improvement on the current situation, even though it still cannot 
handle the case

  class Error(Exception):
def __init__(self, x):
  Exception.__init__(self)
  self.x = x

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Changes by Richard Oudkerk shibt...@gmail.com:


Added file: http://bugs.python.org/file26537/init_args.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

There should be an actual pickling / unpickling test, so that this
doesn't regress by mistake.

Le vendredi 27 juillet 2012 à 11:01 +, Richard Oudkerk a écrit :
 Changes by Richard Oudkerk shibt...@gmail.com:
 
 
 Added file: http://bugs.python.org/file26537/init_args.patch


--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk shibt...@gmail.com added the comment:

ExceptionTests.testAttributes already checks the attributes of unpickled copies 
(using all protocols).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 ExceptionTests.testAttributes already checks the attributes of
 unpickled copies (using all protocols).

Ok, sorry for the noise then.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Brett Cannon]

Brett Cannon br...@python.org added the comment:

Temporarily making this a blocker to see if Georg considers this a bugfix or 
feature.

--
nosy: +benjamin.peterson
priority: critical - release blocker

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk shibt...@gmail.com added the comment:

BTW, BaseException_init() has the code

Py_XDECREF(self-args);
self-args = args;
Py_INCREF(self-args);

Presumably the Py_XDECREF(self-args) would be better replaced by 
Py_CLEAR(self-args)?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 Presumably the Py_XDECREF(self-args) would be better replaced by 
 Py_CLEAR(self-args)?

Ah, yes, definitely.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

Or you could simply Py_INCREF(args) before the Py_XDECREF...

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk shibt...@gmail.com added the comment:

 Or you could simply Py_INCREF(args) before the Py_XDECREF...

But won't self-args point to a broken object while any callbacks triggered by 
Py_XDECREF() are run?

An alternative would be

tmp = self-args;
self-args = args;
Py_INCREF(self-args);
Py_XDECREF(tmp);

As far as I can see the idiom Py_?DECREF(self-...) is rarely safe outside of a 
deallocator unless you are sure the pointed to object has a safe type (or you 
are sure the refcount cannot fall to zero).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 But won't self-args point to a broken object while any callbacks triggered 
 by Py_XDECREF() are run?
 
 An alternative would be
 
 tmp = self-args;
 self-args = args;
 Py_INCREF(self-args);
 Py_XDECREF(tmp);

You are right, that's better.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk shibt...@gmail.com added the comment:

Patch which adds fix for BaseException_init().

Actually this class of problem seem to be quite common.  

BaseException_set_tb() appears to be affected too, as is the code in the 
tutorial which introduces setters.

--
Added file: http://bugs.python.org/file26550/init_args.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk shibt...@gmail.com added the comment:

I see that the originally proposed patch is more or less what I suggested 
above.  Since this has been a critical issue for 5 years, I think such a 
minimal patch should be committed even though it is not a complete solution.

It seems to me that the more complicated patch exception_pickling_26.diff has a 
significant overlap with _common_reduce() in typeobject.c.

The patch actually attempts to be *more* general than _common_reduce() since it 
collects data from tp_getset and tp_members, whereas _common_reduce() only 
collects data from __slots__.  The patch does not seem to take account of 
read-only members/getsetters, which I think would cause exceptions when 
unpickling.

An alternative implementation piggy-backing on _common_reduce() (and using 
__reduce__ rather than __getstate__) would be

def __reduce__(self):
slots = None
func, args, state = object.__reduce__(self, 2)[:3]
if type(state) is tuple:
state, slots = state
newstate = {'args': self.args}
if state:
newstate.update(state)
if slots:
newstate.update(slots)
return func, args, newstate

This deals correctly with slots and works with all protocols.  However, the 
pickles produced can only be unpickled on versions where exceptions are 
new-style classes, ie Python 2.5 and later.

This would have the side effect that __init__() would no longer be called while 
unpickling.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Brett Cannon]

Brett Cannon br...@python.org added the comment:

While I would be happy to see this issue finally closed, but it's up to Georg 
if this goes into 3.3 or not as one could argue it's a feature or a 
bugfix/oversight.

As for the actual fix, classic classes shouldn't hold back a good solution 
since that is a 2.x thing that only affects Python 2.4 which is too old to 
worry about; I think it's an acceptable limitation.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk shibt...@gmail.com added the comment:

I realize now that the idea of using object.__reduce__(..., 2) would not really 
work since many exception classes use non-slot descriptors (unless '__slots__' 
attributes were also added as hints of what to serialize).

I think there are two options simple enough to sneak in to 3.3:

(1) The trivial patch of initially setting self-args in __new__().

(2) Georg's idea of additionally setting a __newargs__ attribute in __new__() 
and using it in __reduce__().  However, I would store __newargs__ directly in 
the struct to avoid always triggering creation of a dict for the instance.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 I think there are two options simple enough to sneak in to 3.3:
 
 (1) The trivial patch of initially setting self-args in __new__().
 
 (2) Georg's idea of additionally setting a __newargs__ attribute in
 __new__() and using it in __reduce__().  However, I would store
 __newargs__ directly in the struct to avoid always triggering creation
 of a dict for the instance.

At this point of the release process, the trivial approach sounds safer
to me (but is it?).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Richard Oudkerk]

Richard Oudkerk shibt...@gmail.com added the comment:

ISTM the simplest approach would be to just set self-args in 
BaseException.__new__() (like in Georg's patch) but to ignore the possibility 
that the user might later set self.args to something stupid wrong:

diff -r 51ac5f06dd04 Objects/exceptions.c
--- a/Objects/exceptions.c  Tue Jul 24 03:45:39 2012 -0700
+++ b/Objects/exceptions.c  Tue Jul 24 22:12:49 2012 +0100
@@ -44,12 +44,17 @@
 self-traceback = self-cause = self-context = NULL;
 self-suppress_context = 0;

-self-args = PyTuple_New(0);
-if (!self-args) {
-Py_DECREF(self);
-return NULL;
+if (!args) {
+args = PyTuple_New(0);
+if (!args) {
+Py_DECREF(self);
+return NULL;
+}
+} else {
+Py_INCREF(args);
 }

+self-args = args;
 return (PyObject *)self;
 }

Certainly it will not work for all cases (like calling a base classes' __init__ 
with different arguments), but it does cover the *very* common case where 
__init__() is defined but does not call the base classes' __init__().

Such a patch is minimally invasive and, as far as I can see, would not break 
currently working code.

Would this be acceptable for a bugfix release?

--
nosy: +sbt

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Éric Araujo]

Changes by Éric Araujo mer...@netwok.org:


--
nosy: +pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Łukasz Langa]

Changes by Łukasz Langa luk...@langa.pl:


--
assignee:  - lukasz.langa
nosy: +lukasz.langa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Zbyszek Szmek]

Changes by Zbyszek Szmek zbys...@in.waw.pl:


--
nosy: +zbysz

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Georg Brandl]

Changes by Georg Brandl ge...@python.org:


--
assignee: georg.brandl - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Faheem Mitha]

Faheem Mitha fah...@faheem.info added the comment:

What is the status on this? It contains to be an
issue. See http://bugs.python.org/issue13751 and
http://bugs.python.org/issue13760

--
nosy: +fmitha

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Tal Einat]

Tal Einat talei...@gmail.com added the comment:

I'd just like to weigh in and say that this is a major issue for me at the 
moment. Not being able to indiscriminately pickle/unpickle exceptions is making 
my parallel-processing work very painful, because of problematic stdlib 
exceptions.

I'm surprised this hasn't been fixed way back in 2.x. FWIW, for my project 
having this fixed in 3.x could be a significant incentive to finally ditch 2.x.

--
nosy: +taleinat

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
versions: +Python 3.3 -Python 3.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Ben Bass]

Ben Bass benpaulb...@googlemail.com added the comment:

Perhaps this should be addressed separately, but subprocess.CalledProcessError 
is subject to this problem (can't be unpickled) (it has separate returncode and 
cmd attributes, but no args).

It's straightforward to conform user-defined Exceptions to including .args and 
having reasonable __init__ functions, but not possible in the case of stdlib 
exceptions.

 import subprocess, pickle
 try:
...   subprocess.check_call('/bin/false')
... except Exception as e:
...   pickle.loads(pickle.dumps(e))
... 
Traceback (most recent call last):
  File stdin, line 2, in module
  File /usr/lib/python3.1/subprocess.py, line 435, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '/bin/false' returned non-zero exit 
status 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File stdin, line 4, in module
  File /usr/lib/python3.1/pickle.py, line 1363, in loads
encoding=encoding, errors=errors).load()
TypeError: __init__() takes at least 3 positional arguments (1 given)

--
nosy: +bpb

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[STINNER Victor]

Changes by STINNER Victor victor.stin...@haypocalc.com:


--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Jason R. Coombs]

Jason R. Coombs jar...@jaraco.com added the comment:

In msg108954, I believe belopolsky is mistaken in stating that it would be 
easy to simply provide custom __getinitargs__ or __reduce__ to support it. It 
appears __getinitargs__ does not work on Python 2.5 or Python 2.7. Exceptions 
of the following class still raise a TypeError on unpickling:

class D(Exception):
Extension with values, init called with no args.
def __init__(self, foo):
self.foo = foo
Exception.__init__(self)

def __getinitargs__(self):
return self.foo,

Using __reduce__ does seem to work. I suspect this is because Exceptions are 
extension types.

I think the fundamental problem is that pickling exceptions does not follow the 
principle of least surprise. In particular:

 - Other built-in objects (dicts, lists, etc) don't require special handling 
(replace Exception with dict in the above example and it works).
 - It's not obvious how to write an Exception subclass that takes additional 
arguments and make it pickleable.
 - Neither the pickle documentation nor the Exception documentation describe 
how pickling is implemented in Exceptions.

Eric has provided some good use cases. Furthermore, it seems counter-intuitive 
to me to pass a subclass' custom arguments to the parent class. Indeed, even 
the official tutorial defines exception classes that are unpickleable 
(http://docs.python.org/tutorial/errors.html#tut-userexceptions).

If the use case is obvious enough that it shows up in the hello world tutorial, 
I don't think there should be any argument that it's not a common use case.

At the very least, there should be a section in the pickle documentation or 
Exception documentation describing how one should make a pickleable subclass. 
What would be better is for Exceptions to behave like most other classes when 
being pickled.

--
nosy: +jaraco

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Jason R. Coombs]

Jason R. Coombs jar...@jaraco.com added the comment:

After some further reading, I found that PEP-352 
(http://www.python.org/dev/peps/pep-0352/) explicitly states that Including 
programmatic information (e.g., an error code number) should be stored as a 
separate attribute in a subclass [and not in the args attribute]. The 
parameter to Exception.__init__ should always be a single, human-readable 
string. The default pickler doesn't handle this officially-prescribed use-case 
without overriding __reduce__.

class F(Exception):
def __init__(self, message, code):
Exception.__init__(self, message)
self.code = code

Of course, as belopolsky observed, __repr__ must also be overridden.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Alexander Belopolsky]

Alexander Belopolsky belopol...@users.sourceforge.net added the comment:

On Wed, Sep 22, 2010 at 9:39 AM, Jason R. Coombs rep...@bugs.python.org wrote:
 .. It appears __getinitargs__ does not work on Python 2.5 or Python 2.7.

Yes,  __getinitargs__  is only used for old style classes.  I was
wrong on that point.

 Exceptions of the following class still raise a TypeError on unpickling:

 class D(Exception):
    Extension with values, init called with no args.
    def __init__(self, foo):
        self.foo = foo
        Exception.__init__(self)

    def __getinitargs__(self):
        return self.foo,


The problem with your D class is that it does not provide correct args
attribute which is expected from an exception class:

()

 Using __reduce__ does seem to work. I suspect this is because Exceptions are 
 extension types.


There are two ways to fix this class.  Both fix the args issue as well:

1. Pass foo to Exception.__init__  like this: Exception.__init__(self,
foo) in D.__init__.

2. Explicitly initialize self.args: self.args   = foo,

 I think the fundamental problem is that pickling exceptions does not follow 
 the principle of least surprise. In particular:

  - Other built-in objects (dicts, lists, etc) don't require special handling 
 (replace Exception with dict in the above example and it works).

Other built-in objects don't provide an API for retrieving their init arguments.

  - It's not obvious how to write an Exception subclass that takes additional 
 arguments and make it pickleable.

AFAICT, this is python subclassing 101: if base class __init__ uses
arguments, they should be passed to it by subclass' __init__.

  - Neither the pickle documentation nor the Exception documentation describe 
 how pickling is implemented in Exceptions.


Exception documentation may be improved by adding a section on
subclassing.  Note that the args argument was not even mentioned in
2.7 documentation, so some discussion of its role may be helpful
somewhere.

 Eric has provided some good use cases. Furthermore, it seems 
 counter-intuitive to me to pass a subclass' custom
 arguments to the parent class. Indeed, even the official tutorial defines 
 exception classes that are unpickleable
 (http://docs.python.org/tutorial/errors.html#tut-userexceptions).


Well, the tutorial examples should probably be changed.  In these
examples base class __init__ is not called at all which is probably
not a good style.

 If the use case is obvious enough that it shows up in the hello world 
 tutorial, I don't think
 there should be any argument that it's not a common use case.


I am not arguing against simplifying Exception subclassing.  I just
don't see an easy solution that would not promote subclasses with
unusable args attribute.  I also disagree with this issue classified
as a bug.  It may be a valid feature request, but not a bug.

In any case, no proponent of this feature has come up with a patch for
3.2 so far and in my view, this would be a prerequisite for moving
this forward.

 At the very least, there should be a section in the pickle documentation or 
 Exception
 documentation describing how one should make a pickleable subclass. ..

I agree, but again someone has to step in to write such section.
Improving documentation may also be the only solution for the 2.x
series.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Mark Lawrence]

Changes by Mark Lawrence breamore...@yahoo.co.uk:


--
versions: +Python 2.7, Python 3.2 -Python 2.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Alexander Belopolsky]

Alexander Belopolsky belopol...@users.sourceforge.net added the comment:

The case in msg76028 are probably not pointing to a bug.  If subclass' __init__ 
passes its args to the base class __init__ (as it probably should), pickling 
works: 

class E(Exception):
Extension with values, init called with args.
def __init__(self, foo):
self.foo = foo
Exception.__init__(self, foo)

 from pickle import *
 loads(dumps(E(1)))
E(1,)
 _.foo
1

What would be a use case for not setting args?  Even if there was a valid use 
case for it, it would be easy to simply provide custom __getinitargs__ or 
__reduce__ to support it.

Note that with msg76028 definitions, eval(repr(C('foo'))) also raises a 
TypeError, so properly supporting args hiding in exception subclasses would 
probably involve overriding __repr__ as well.

--
nosy: +belopolsky

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Eric Huss]

Eric Huss e...@huss.org added the comment:

Alexander, the use case I was involved with was an RPC system which allowed 
exceptions to propagate over the connection.  In this case, you do not have 
absolute control over which exceptions may be raised, or who wrote the code 
that is raising the exception.  There are many cases, even in the standard 
library, where people do not pass all arguments to the base exception class.  
Some of these are probably mistakes, but in general pickle shouldn't fail on 
otherwise legitimate objects.

There are other cases where passing all arguments up is not wanted, or at least 
cumbersome.  If you have multiple levels of inheritance, and subclasses add 
additional arguments to the init, they wouldn't have a way to include those 
arguments to the base class unless all classes were written with *args in the 
init function, which many people would not know or remember to do.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Tres Seaver]

Tres Seaver tsea...@agendaless.com added the comment:

The attached patch adds Mark's examples to test_pickle as a failing test.

--
nosy: +tseaver
Added file: http://bugs.python.org/file17509/issue1692335-tests.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Kyle VanderBeek]

Kyle VanderBeek ky...@kylev.com added the comment:

Ping?  Is anyone working on this?  Have Eric's concerns been addressed and can 
we pickle/unpickle all exceptions yet?

--
nosy: +kylev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Daniel Diniz]

Changes by Daniel Diniz aja...@gmail.com:


--
stage:  - patch review
type:  - behavior
versions: +Python 3.1 -Python 2.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Eric Huss]

Eric Huss [EMAIL PROTECTED] added the comment:

I'm disappointed to see this closed.  Exception pickling is still broken
in some cases in 2.6.

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Eric Huss]

Eric Huss [EMAIL PROTECTED] added the comment:

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.

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Benjamin Peterson]

Changes by Benjamin Peterson [EMAIL PROTECTED]:


--
nosy:  -benjamin.peterson

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Guido van Rossum]

Guido van Rossum [EMAIL PROTECTED] added the comment:

OK, reopening. Can you post an example that fails today?

--
resolution: out of date - 
status: closed - open

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Ziga Seilnacht]

Ziga Seilnacht [EMAIL PROTECTED] added the comment:

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.

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Guido van Rossum]

Changes by Guido van Rossum [EMAIL PROTECTED]:


--
resolution:  - out of date
status: open - closed

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Benjamin Peterson]

Benjamin Peterson [EMAIL PROTECTED] added the comment:

How is this coming? Can we apply this to 2.6?

--
nosy: +benjamin.peterson

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Alexandre Vassalotti]

Changes by Alexandre Vassalotti:


--
nosy: +alexandre.vassalotti

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Martin v. Löwis]

Martin v. Löwis added the comment:

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.

--
nosy: +loewis

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Jaroslav Pachola]

Jaroslav Pachola added the comment:

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.

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Guido van Rossum]

Guido van Rossum added the comment:

Understood.  Can you get other developers on python-dev to weigh in? 
Maybe I am over-estimating the danger.

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Guido van Rossum]

Guido van Rossum added the comment:

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.)

--
nosy: +gvanrossum

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Facundo Batista]

Facundo Batista added the comment:

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

--
nosy: +facundobatista

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Jaroslav Pachola]

Jaroslav Pachola added the comment:

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.

--
nosy: +jarpa
Added file: http://bugs.python.org/file9198/exception_pickling_26.diff

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Brett Cannon]

Changes by Brett Cannon:


--
nosy: +brett.cannon

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Sean Reifschneider]

Sean Reifschneider added the comment:

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?

--
assignee:  - georg.brandl
nosy: +jafo

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Sean Reifschneider]

Changes by Sean Reifschneider:


_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Sean Reifschneider]

Changes by Sean Reifschneider:


_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1692335] Fix exception pickling: Move initial args assignment to BaseException.__new__

[Georg Brandl]

Georg Brandl added the comment:

Raising priority.

--
priority: normal - urgent
severity: normal - major
title: Move initial args assignment to BaseException.__new__ - Fix exception 
pickling: Move initial args assignment to BaseException.__new__
versions: +Python 2.6

_
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1692335
_
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com