From 7b97f04744f47c8e4ec9cfbb88c740001b9abde2 Mon Sep 17 00:00:00 2001 From: Sergey Fedoseev Date: Thu, 30 Aug 2018 23:52:44 +0500 Subject: [PATCH 1/4] bpo-34574: Prevent OrderedDict iterators from exhaustion during pickling. --- Lib/test/test_ordered_dict.py | 20 +++++++++++ .../2018-09-04-09-32-54.bpo-34574.X4RwYI.rst | 2 ++ Objects/odictobject.c | 35 ++++--------------- 3 files changed, 29 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-09-04-09-32-54.bpo-34574.X4RwYI.rst diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 20efe3729de838..3c1cf8002fb909 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -732,6 +732,26 @@ def test_key_change_during_iteration(self): del od['c'] self.assertEqual(list(od), list('bdeaf')) + def test_iterators_pickling(self): + OrderedDict = self.OrderedDict + pairs = [('c', 1), ('b', 2), ('a', 3), ('d', 4), ('e', 5), ('f', 6)] + od = OrderedDict(pairs) + + expected = ( + ('keys', [k for k, v in pairs[:1]]), + ('values', [v for k, v in pairs[:1]]), + ('items', pairs[:1]), + ) + for method_name, items in expected: + meth = getattr(od, method_name) + with self.subTest(method_name=method_name): + for i in range(pickle.HIGHEST_PROTOCOL + 1): + it = iter(meth()) + next(it) + p = pickle.dumps(it, i) + unpickled = pickle.loads(p) + self.assertEqual(list(it), list(unpickled)) + class PurePythonOrderedDictSubclassTests(PurePythonOrderedDictTests): diff --git a/Misc/NEWS.d/next/Library/2018-09-04-09-32-54.bpo-34574.X4RwYI.rst b/Misc/NEWS.d/next/Library/2018-09-04-09-32-54.bpo-34574.X4RwYI.rst new file mode 100644 index 00000000000000..de718ad1e6f102 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-09-04-09-32-54.bpo-34574.X4RwYI.rst @@ -0,0 +1,2 @@ +OrderedDict iterators are not exhausted during pickling anymore. Patch by +Sergey Fedoseev. diff --git a/Objects/odictobject.c b/Objects/odictobject.c index 353afd23d6b5e8..5a9516b2d1ceb1 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -1837,38 +1837,17 @@ PyDoc_STRVAR(reduce_doc, "Return state information for pickling"); static PyObject * odictiter_reduce(odictiterobject *di) { - PyObject *list, *iter; - - list = PyList_New(0); - if (!list) - return NULL; + /* copy the iterator state */ + odictiterobject tmp = *di; + Py_XINCREF(tmp.di_odict); /* iterate the temporary into a list */ - for(;;) { - PyObject *element = odictiter_iternext(di); - if (element) { - if (PyList_Append(list, element)) { - Py_DECREF(element); - Py_DECREF(list); - return NULL; - } - Py_DECREF(element); - } - else { - /* done iterating? */ - break; - } - } - if (PyErr_Occurred()) { - Py_DECREF(list); - return NULL; - } - iter = _PyObject_GetBuiltin("iter"); - if (iter == NULL) { - Py_DECREF(list); + PyObject *list = PySequence_List((PyObject*)&tmp); + Py_XDECREF(tmp.di_odict); + if (list == NULL) { return NULL; } - return Py_BuildValue("N(N)", iter, list); + return Py_BuildValue("N(N)", _PyObject_GetBuiltin("iter"), list); } static PyMethodDef odictiter_methods[] = { From 438c739976bccbe7e33270d41e895334413e31c1 Mon Sep 17 00:00:00 2001 From: Sergey Fedoseev Date: Tue, 4 Sep 2018 09:51:59 +0500 Subject: [PATCH 2/4] Removed unused code. Use subTest also for pickling protocol. --- Lib/test/test_ordered_dict.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 3c1cf8002fb909..120fefeeff00c7 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -737,15 +737,10 @@ def test_iterators_pickling(self): pairs = [('c', 1), ('b', 2), ('a', 3), ('d', 4), ('e', 5), ('f', 6)] od = OrderedDict(pairs) - expected = ( - ('keys', [k for k, v in pairs[:1]]), - ('values', [v for k, v in pairs[:1]]), - ('items', pairs[:1]), - ) - for method_name, items in expected: + for method_name in ('keys', 'values', 'items'): meth = getattr(od, method_name) - with self.subTest(method_name=method_name): - for i in range(pickle.HIGHEST_PROTOCOL + 1): + for i in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(method_name=method_name, protocol=i): it = iter(meth()) next(it) p = pickle.dumps(it, i) From 758530dc4bbf851a2cf98f83913d58e62a12524c Mon Sep 17 00:00:00 2001 From: Sergey Fedoseev Date: Tue, 4 Sep 2018 10:43:18 +0500 Subject: [PATCH 3/4] Improve testing. --- Lib/test/test_ordered_dict.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 120fefeeff00c7..bcb437f18ee478 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -739,13 +739,15 @@ def test_iterators_pickling(self): for method_name in ('keys', 'values', 'items'): meth = getattr(od, method_name) + expected = list(meth())[1:] for i in range(pickle.HIGHEST_PROTOCOL + 1): with self.subTest(method_name=method_name, protocol=i): it = iter(meth()) next(it) p = pickle.dumps(it, i) unpickled = pickle.loads(p) - self.assertEqual(list(it), list(unpickled)) + self.assertEqual(list(unpickled), expected) + self.assertEqual(list(it), expected) class PurePythonOrderedDictSubclassTests(PurePythonOrderedDictTests): From d6e9ba8a7169a30748ce8e9cc4280aa6848d2104 Mon Sep 17 00:00:00 2001 From: Sergey Fedoseev Date: Tue, 4 Sep 2018 12:49:05 +0500 Subject: [PATCH 4/4] Fixed double DECREF. --- Objects/odictobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Objects/odictobject.c b/Objects/odictobject.c index 5a9516b2d1ceb1..27457d3cc85cb8 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -1840,10 +1840,12 @@ odictiter_reduce(odictiterobject *di) /* copy the iterator state */ odictiterobject tmp = *di; Py_XINCREF(tmp.di_odict); + Py_XINCREF(tmp.di_current); /* iterate the temporary into a list */ PyObject *list = PySequence_List((PyObject*)&tmp); Py_XDECREF(tmp.di_odict); + Py_XDECREF(tmp.di_current); if (list == NULL) { return NULL; }