classification
Title: itertools.accumulate __reduce__/__setstate__ bug
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: alexandre.vassalotti, arigo, kristjan.jonsson, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-11-24 08:07 by arigo, last changed 2016-03-06 12:34 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
copy_state_is_false.patch serhiy.storchaka, 2015-11-24 16:37 review
itertools_accumulate_state_is_none.patch serhiy.storchaka, 2015-12-07 13:18 review
Messages (14)
msg255252 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2015-11-24 08:07
itertools.accumulate() has got methods __reduce__() and __setstate__() which fail at saving/restoring the state in all cases.  Example:

   >>> a = itertools.accumulate([0.0, 42])
   >>> next(a)
   0.0
   >>> b = copy.deepcopy(a)
   >>> next(a)
   42.0
   >>> next(b)
   42           # should have been the same as the previous result

More precisely, the problem occurs when the C-level "total" field has some value whose truth-value is false.  Then __reduce__/__setstate__ will not restore the value of this field, but keep it NULL.  That's why in the example above the intermediate total of 0.0 is forgotten, and the next(b) call will again just pick the next value without doing any addition.  (The problem can hurt more than in this example if we use a custom function instead of addition.)

A fix is easy but, as far as I can tell, it requires breaking the pickled format of accumulate() iterators that have already started.
msg255280 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-24 16:37
__reduce__() and __setstate__() methods of itertools.accumulate() look correct. The problem is not in itertools.accumulate(), but in the copy module. It uses the same __reduce__ protocol as pickle, but in different way. In the pickle module (in both Python and C implementations) state value is ignored only if it is None. In the copy module any state with boolean value is False is ignored.

Proposed patch fixes the copy module.
msg255329 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2015-11-25 09:59
Ok, then with pickle you can have the same problem but only with None-vs-no-total.  Here is an artificial example:

import itertools, pickle

def foo(a, b):
    print(a, b)

a = itertools.accumulate([3, 4, 5], foo)
next(a)
next(a)   # prints: 3, 4

b = pickle.loads(pickle.dumps(a))

next(a)   # prints: None, 5
next(b)   # foo() is not called at all
msg255336 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-25 11:23
In this case yes, you are unlucky. But this is an artificial example, and I don't think this happens in any real code.

However specific meaning of None should be documented (there are other wrong words about __getstate__).
msg255622 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-11-30 15:38
New changeset 095d21df9374 by Serhiy Storchaka in branch '3.4':
Issue #25718: Fixed copying object with state with boolean value is false.
https://hg.python.org/cpython/rev/095d21df9374

New changeset 3d39e1e2dcb3 by Serhiy Storchaka in branch '2.7':
Issue #25718: Fixed copying object with state with boolean value is false.
https://hg.python.org/cpython/rev/3d39e1e2dcb3

New changeset fecb07050aae by Serhiy Storchaka in branch '3.5':
Issue #25718: Fixed copying object with state with boolean value is false.
https://hg.python.org/cpython/rev/fecb07050aae

New changeset 962086677306 by Serhiy Storchaka in branch 'default':
Issue #25718: Fixed copying object with state with boolean value is false.
https://hg.python.org/cpython/rev/962086677306
msg255625 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-30 19:58
The bug in the copy module is fixed. As for accumulate's __reduce__, I don't think we can do something.
msg256058 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2015-12-07 12:07
This could be fixed by saving the accumulate state in a tuple.  It would break protocol, though.  I don't recall the rules for backwards compatibility of pickles.
I've argued before that the state of runtime structures such as generators is so intimately tied with the currently executing python that we shouldn't worry to much about it.  This sort of stuff is used for caches, IPC, and so on.  But it's not my call.
msg256060 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-07 13:18
Well, we can fix this without breaking the protocol. The accumulate() object with state is None can be reconstructed as islice(accumulate(chain([None], iterator)), 1, None).

Proposed patch implements this solution.
msg256695 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-18 16:51
What would you say about the patch Kristján?
msg258577 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-01-19 10:09
Ping.
msg260997 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-02-29 09:12
Ping again.
msg261237 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-06 07:20
Raymond, could you please look at the patch?
msg261242 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-03-06 10:42
+1
msg261247 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-06 12:02
New changeset f3c54cbac3de by Serhiy Storchaka in branch '3.5':
Issue #25718: Fixed pickling and copying the accumulate() iterator with total is None.
https://hg.python.org/cpython/rev/f3c54cbac3de

New changeset 49c035b30e18 by Serhiy Storchaka in branch 'default':
Issue #25718: Fixed pickling and copying the accumulate() iterator with total is None.
https://hg.python.org/cpython/rev/49c035b30e18
History
Date User Action Args
2016-03-06 12:34:39serhiy.storchakasetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: - Python 2.7, Python 3.4
2016-03-06 12:02:59python-devsetmessages: + msg261247
2016-03-06 10:42:26rhettingersetassignee: rhettinger -> serhiy.storchaka
messages: + msg261242
2016-03-06 07:20:55serhiy.storchakasetassignee: kristjan.jonsson -> rhettinger

messages: + msg261237
nosy: + rhettinger
2016-02-29 09:12:11serhiy.storchakasetmessages: + msg260997
2016-01-19 10:09:14serhiy.storchakasetmessages: + msg258577
2015-12-18 16:51:04serhiy.storchakasetmessages: + msg256695
2015-12-07 13:18:46serhiy.storchakasetfiles: + itertools_accumulate_state_is_none.patch

messages: + msg256060
2015-12-07 12:07:39kristjan.jonssonsetmessages: + msg256058
2015-11-30 19:58:46serhiy.storchakasetmessages: + msg255625
2015-11-30 15:38:07python-devsetnosy: + python-dev
messages: + msg255622
2015-11-25 11:23:53serhiy.storchakasetmessages: + msg255336
2015-11-25 09:59:06arigosetmessages: + msg255329
2015-11-24 16:37:25serhiy.storchakasetfiles: + copy_state_is_false.patch
keywords: + patch
2015-11-24 16:37:09serhiy.storchakasetversions: + Python 2.7, Python 3.4, Python 3.5
nosy: + alexandre.vassalotti, serhiy.storchaka

messages: + msg255280

components: + Library (Lib), - Extension Modules
stage: patch review
2015-11-24 08:52:52rhettingersetassignee: kristjan.jonsson

nosy: + kristjan.jonsson
2015-11-24 08:07:23arigocreate