From 5e08ff256cd4fa6642546e408742e7eecc41212f Mon Sep 17 00:00:00 2001 From: Charles Machalow Date: Fri, 7 Apr 2023 13:21:50 -0700 Subject: [PATCH 1/6] Pickle the keyword attributes of AttributeError --- Objects/exceptions.c | 45 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/Objects/exceptions.c b/Objects/exceptions.c index a355244cf997e6..ad87dc3608c5e3 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -2282,6 +2282,47 @@ AttributeError_traverse(PyAttributeErrorObject *self, visitproc visit, void *arg return BaseException_traverse((PyBaseExceptionObject *)self, visit, arg); } +/* Pickling support */ +static PyObject * +AttributeError_getstate(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject *dict = ((PyAttributeErrorObject *)self)->dict; + if (self->name || self->obj || self->args) { + dict = dict ? PyDict_Copy(dict) : PyDict_New(); + if (dict == NULL) + return NULL; + if (self->name && PyDict_SetItemString(dict, "name", self->name) < 0) { + Py_DECREF(dict); + return NULL; + } + if (self->obj && PyDict_SetItemString(dict, "obj", self->obj) < 0) { + Py_DECREF(dict); + return NULL; + } + if (self->args && PyDict_SetItemString(dict, "args", self->args) < 0) { + Py_DECREF(dict); + return NULL; + } + return dict; + } + else if (dict) { + return Py_NewRef(dict); + } + else { + Py_RETURN_NONE; + } +} + +static PyObject * +AttributeError_reduce(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject *state = AttributeError_getstate(self, NULL); + if (state == NULL) + return NULL; + + return PyTuple_Pack(3, Py_TYPE(self), self->args, state); +} + static PyMemberDef AttributeError_members[] = { {"name", T_OBJECT, offsetof(PyAttributeErrorObject, name), 0, PyDoc_STR("attribute name")}, {"obj", T_OBJECT, offsetof(PyAttributeErrorObject, obj), 0, PyDoc_STR("object")}, @@ -2289,7 +2330,9 @@ static PyMemberDef AttributeError_members[] = { }; static PyMethodDef AttributeError_methods[] = { - {NULL} /* Sentinel */ + {"__getstate__", (PyCFunction)AttributeError_getstate, METH_NOARGS}, + {"__reduce__", (PyCFunction)AttributeError_reduce, METH_NOARGS }, + {NULL} }; ComplexExtendsException(PyExc_Exception, AttributeError, From 1102306e1d37d15580f7a1c33e71261e292cefcc Mon Sep 17 00:00:00 2001 From: Charles Machalow Date: Sat, 8 Apr 2023 18:33:56 -0700 Subject: [PATCH 2/6] Only pickle names/args for AttributeError, don't do obj --- Lib/test/test_exceptions.py | 62 +++++++++++++++++++++---------------- Objects/exceptions.c | 3 ++ 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 684e888f08c778..3bb012d0ef54a3 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -416,45 +416,45 @@ def testAttributes(self): # test that exception attributes are happy exceptionList = [ - (BaseException, (), {'args' : ()}), - (BaseException, (1, ), {'args' : (1,)}), - (BaseException, ('foo',), + (BaseException, (), {}, {'args' : ()}), + (BaseException, (1, ), {}, {'args' : (1,)}), + (BaseException, ('foo',), {}, {'args' : ('foo',)}), - (BaseException, ('foo', 1), + (BaseException, ('foo', 1), {}, {'args' : ('foo', 1)}), - (SystemExit, ('foo',), + (SystemExit, ('foo',), {}, {'args' : ('foo',), 'code' : 'foo'}), - (OSError, ('foo',), + (OSError, ('foo',), {}, {'args' : ('foo',), 'filename' : None, 'filename2' : None, 'errno' : None, 'strerror' : None}), - (OSError, ('foo', 'bar'), + (OSError, ('foo', 'bar'), {}, {'args' : ('foo', 'bar'), 'filename' : None, 'filename2' : None, 'errno' : 'foo', 'strerror' : 'bar'}), - (OSError, ('foo', 'bar', 'baz'), + (OSError, ('foo', 'bar', 'baz'), {}, {'args' : ('foo', 'bar'), 'filename' : 'baz', 'filename2' : None, 'errno' : 'foo', 'strerror' : 'bar'}), - (OSError, ('foo', 'bar', 'baz', None, 'quux'), + (OSError, ('foo', 'bar', 'baz', None, 'quux'), {}, {'args' : ('foo', 'bar'), 'filename' : 'baz', 'filename2': 'quux'}), - (OSError, ('errnoStr', 'strErrorStr', 'filenameStr'), + (OSError, ('errnoStr', 'strErrorStr', 'filenameStr'), {}, {'args' : ('errnoStr', 'strErrorStr'), 'strerror' : 'strErrorStr', 'errno' : 'errnoStr', 'filename' : 'filenameStr'}), - (OSError, (1, 'strErrorStr', 'filenameStr'), + (OSError, (1, 'strErrorStr', 'filenameStr'), {}, {'args' : (1, 'strErrorStr'), 'errno' : 1, 'strerror' : 'strErrorStr', 'filename' : 'filenameStr', 'filename2' : None}), - (SyntaxError, (), {'msg' : None, 'text' : None, + (SyntaxError, (), {}, {'msg' : None, 'text' : None, 'filename' : None, 'lineno' : None, 'offset' : None, 'end_offset': None, 'print_file_and_line' : None}), - (SyntaxError, ('msgStr',), + (SyntaxError, ('msgStr',), {}, {'args' : ('msgStr',), 'text' : None, 'print_file_and_line' : None, 'msg' : 'msgStr', 'filename' : None, 'lineno' : None, 'offset' : None, 'end_offset': None}), (SyntaxError, ('msgStr', ('filenameStr', 'linenoStr', 'offsetStr', - 'textStr', 'endLinenoStr', 'endOffsetStr')), + 'textStr', 'endLinenoStr', 'endOffsetStr')), {}, {'offset' : 'offsetStr', 'text' : 'textStr', 'args' : ('msgStr', ('filenameStr', 'linenoStr', 'offsetStr', 'textStr', @@ -464,7 +464,7 @@ def testAttributes(self): 'end_lineno': 'endLinenoStr', 'end_offset': 'endOffsetStr'}), (SyntaxError, ('msgStr', 'filenameStr', 'linenoStr', 'offsetStr', 'textStr', 'endLinenoStr', 'endOffsetStr', - 'print_file_and_lineStr'), + 'print_file_and_lineStr'), {}, {'text' : None, 'args' : ('msgStr', 'filenameStr', 'linenoStr', 'offsetStr', 'textStr', 'endLinenoStr', 'endOffsetStr', @@ -472,38 +472,40 @@ def testAttributes(self): 'print_file_and_line' : None, 'msg' : 'msgStr', 'filename' : None, 'lineno' : None, 'offset' : None, 'end_lineno': None, 'end_offset': None}), - (UnicodeError, (), {'args' : (),}), + (UnicodeError, (), {}, {'args' : (),}), (UnicodeEncodeError, ('ascii', 'a', 0, 1, - 'ordinal not in range'), + 'ordinal not in range'), {}, {'args' : ('ascii', 'a', 0, 1, 'ordinal not in range'), 'encoding' : 'ascii', 'object' : 'a', 'start' : 0, 'reason' : 'ordinal not in range'}), (UnicodeDecodeError, ('ascii', bytearray(b'\xff'), 0, 1, - 'ordinal not in range'), + 'ordinal not in range'), {}, {'args' : ('ascii', bytearray(b'\xff'), 0, 1, 'ordinal not in range'), 'encoding' : 'ascii', 'object' : b'\xff', 'start' : 0, 'reason' : 'ordinal not in range'}), (UnicodeDecodeError, ('ascii', b'\xff', 0, 1, - 'ordinal not in range'), + 'ordinal not in range'), {}, {'args' : ('ascii', b'\xff', 0, 1, 'ordinal not in range'), 'encoding' : 'ascii', 'object' : b'\xff', 'start' : 0, 'reason' : 'ordinal not in range'}), - (UnicodeTranslateError, ("\u3042", 0, 1, "ouch"), + (UnicodeTranslateError, ("\u3042", 0, 1, "ouch"), {}, {'args' : ('\u3042', 0, 1, 'ouch'), 'object' : '\u3042', 'reason' : 'ouch', 'start' : 0, 'end' : 1}), - (NaiveException, ('foo',), + (NaiveException, ('foo',), {}, {'args': ('foo',), 'x': 'foo'}), - (SlottedNaiveException, ('foo',), + (SlottedNaiveException, ('foo',), {}, {'args': ('foo',), 'x': 'foo'}), + (AttributeError, ('foo',), dict(name='name', obj='obj'), + dict(args=('foo',), name='name', obj='obj')), ] try: # More tests are in test_WindowsError exceptionList.append( - (WindowsError, (1, 'strErrorStr', 'filenameStr'), + (WindowsError, (1, 'strErrorStr', 'filenameStr'), {}, {'args' : (1, 'strErrorStr'), 'strerror' : 'strErrorStr', 'winerror' : None, 'errno' : 1, @@ -512,11 +514,12 @@ def testAttributes(self): except NameError: pass - for exc, args, expected in exceptionList: + for exc, args, kwargs, expected in exceptionList: try: - e = exc(*args) + e = exc(*args, **kwargs) except: - print("\nexc=%r, args=%r" % (exc, args), file=sys.stderr) + print("\nexc=%r, args=%r, kwargs=%r" % + (exc, args, kwargs), file=sys.stderr) # raise else: # Verify module name @@ -539,7 +542,12 @@ def testAttributes(self): new = p.loads(s) for checkArgName in expected: got = repr(getattr(new, checkArgName)) - want = repr(expected[checkArgName]) + if exc == AttributeError and checkArgName == 'obj': + # See GH-103352, we're not pickling + # obj at this point. So verify it's None. + want = repr(None) + else: + want = repr(expected[checkArgName]) self.assertEqual(got, want, 'pickled "%r", attribute "%s' % (e, checkArgName)) diff --git a/Objects/exceptions.c b/Objects/exceptions.c index ad87dc3608c5e3..996ba873444884 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -2295,10 +2295,13 @@ AttributeError_getstate(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignore Py_DECREF(dict); return NULL; } + /* We specifically are not pickling the obj attribute since there are many + cases where it is unlikely to be picklable. See GH-103352. if (self->obj && PyDict_SetItemString(dict, "obj", self->obj) < 0) { Py_DECREF(dict); return NULL; } + */ if (self->args && PyDict_SetItemString(dict, "args", self->args) < 0) { Py_DECREF(dict); return NULL; From 9873fcc95e2cf5fb2d404aee0eedae7bdc62b035 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 9 Apr 2023 04:30:04 +0000 Subject: [PATCH 3/6] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst new file mode 100644 index 00000000000000..c78115c2e54e6e --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst @@ -0,0 +1 @@ +AttributeError now brings the name attribute when pickled/unpickled. From 96d98861471ea8f6810380d30a6e37bf2458a657 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 9 May 2023 22:55:36 -0700 Subject: [PATCH 4/6] ReSTify the NEWS entry. --- .../2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst index c78115c2e54e6e..793f02c2afdcff 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst @@ -1 +1 @@ -AttributeError now brings the name attribute when pickled/unpickled. +:exc:`AttributeError` now retains the ``name`` attribute when pickled and unpickled. From e08e2175fac239a1ff8a4e701e31090ca2c669d1 Mon Sep 17 00:00:00 2001 From: Charles Machalow Date: Wed, 10 May 2023 19:12:28 -0700 Subject: [PATCH 5/6] PR feedback --- Lib/test/test_exceptions.py | 3 +-- Objects/exceptions.c | 6 +----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 91fb1e3a558abe..f3554f1c4bb3f3 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -519,8 +519,7 @@ def testAttributes(self): try: e = exc(*args, **kwargs) except: - print("\nexc=%r, args=%r, kwargs=%r" % - (exc, args, kwargs), file=sys.stderr) + print(f"\nexc={exc!r}, args={args!r}", file=sys.stderr) # raise else: # Verify module name diff --git a/Objects/exceptions.c b/Objects/exceptions.c index f6405b9a6b7ded..efe33e33e279ba 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -2292,7 +2292,7 @@ static PyObject * AttributeError_getstate(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *dict = ((PyAttributeErrorObject *)self)->dict; - if (self->name || self->obj || self->args) { + if (self->name || self->args) { dict = dict ? PyDict_Copy(dict) : PyDict_New(); if (dict == NULL) return NULL; @@ -2302,10 +2302,6 @@ AttributeError_getstate(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignore } /* We specifically are not pickling the obj attribute since there are many cases where it is unlikely to be picklable. See GH-103352. - if (self->obj && PyDict_SetItemString(dict, "obj", self->obj) < 0) { - Py_DECREF(dict); - return NULL; - } */ if (self->args && PyDict_SetItemString(dict, "args", self->args) < 0) { Py_DECREF(dict); From 18598e2c7e6c367a6d237f3d41992939481af6b7 Mon Sep 17 00:00:00 2001 From: Charles Machalow Date: Thu, 11 May 2023 19:36:21 -0700 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Objects/exceptions.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Objects/exceptions.c b/Objects/exceptions.c index efe33e33e279ba..59c63f4aa44958 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -2294,8 +2294,9 @@ AttributeError_getstate(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignore PyObject *dict = ((PyAttributeErrorObject *)self)->dict; if (self->name || self->args) { dict = dict ? PyDict_Copy(dict) : PyDict_New(); - if (dict == NULL) + if (dict == NULL) { return NULL; + } if (self->name && PyDict_SetItemString(dict, "name", self->name) < 0) { Py_DECREF(dict); return NULL; @@ -2312,17 +2313,16 @@ AttributeError_getstate(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignore else if (dict) { return Py_NewRef(dict); } - else { - Py_RETURN_NONE; - } + Py_RETURN_NONE; } static PyObject * AttributeError_reduce(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *state = AttributeError_getstate(self, NULL); - if (state == NULL) + if (state == NULL) { return NULL; + } return PyTuple_Pack(3, Py_TYPE(self), self->args, state); }