classification
Title: crash when deleting one pair from tee()
Type: crash Stage: resolved
Components: Extension Modules Versions: Python 3.4, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: PyryP, amaury.forgeotdarc, ezio.melotti, georg.brandl, haypo, pitrou, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: needs review, patch

Created on 2011-11-22 16:10 by PyryP, last changed 2013-01-26 09:56 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
itertools_tee_nonrecursive_clear.patch serhiy.storchaka, 2012-10-15 06:53 review
itertools_tee_nonrecursive_clear_2.patch serhiy.storchaka, 2012-12-28 17:26 review
Messages (14)
msg148124 - (view) Author: Pyry Pakkanen (PyryP) Date: 2011-11-22 16:10
Running the following results in a Segmentation fault on Ubuntu 11.10 64-bit with both python and python3.

from itertools import *
c = count()
a,b = tee(c)
for i in range(10000000):
 next(a)
del(b)
msg148125 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-11-22 16:22
tee() uses a linked-list of teedataobject. This list is destroyed by recursive calls to teedataobject_dealloc().

Extract of the gdb trace:

#5  0x08171a8e in teedataobject_clear (tdo=0xad21b394) at ./Modules/itertoolsmodule.c:412
#6  0x08171b39 in teedataobject_dealloc (tdo=0xad21b394) at ./Modules/itertoolsmodule.c:421
#7  0x0805ed3d in _Py_Dealloc (op=<itertools.tee_dataobject at remote 0xad21b394>) at Objects/object.c:1676
#8  0x08171b16 in teedataobject_clear (tdo=0xad21b274) at ./Modules/itertoolsmodule.c:413
#9  0x08171b39 in teedataobject_dealloc (tdo=0xad21b274) at ./Modules/itertoolsmodule.c:421
#10 0x0805ed3d in _Py_Dealloc (op=<itertools.tee_dataobject at remote 0xad21b274>) at Objects/object.c:1676
#11 0x08171b16 in teedataobject_clear (tdo=0xad21b154) at ./Modules/itertoolsmodule.c:413
#12 0x08171b39 in teedataobject_dealloc (tdo=0xad21b154) at ./Modules/itertoolsmodule.c:421
#13 0x0805ed3d in _Py_Dealloc (op=<itertools.tee_dataobject at remote 0xad21b154>) at Objects/object.c:1676
#14 0x08171b16 in teedataobject_clear (tdo=0xad21b034) at ./Modules/itertoolsmodule.c:413
#15 0x08171b39 in teedataobject_dealloc (tdo=0xad21b034) at ./Modules/itertoolsmodule.c:421
#16 0x0805ed3d in _Py_Dealloc (op=<itertools.tee_dataobject at remote 0xad21b034>) at Objects/object.c:1676
#17 0x08171b16 in teedataobject_clear (tdo=0xad292ed4) at ./Modules/itertoolsmodule.c:413
#18 0x08171b39 in teedataobject_dealloc (tdo=0xad292ed4) at ./Modules/itertoolsmodule.c:421
#19 0x0805ed3d in _Py_Dealloc (op=<itertools.tee_dataobject at remote 0xad292ed4>) at Objects/object.c:1676
#20 0x08171b16 in teedataobject_clear (tdo=0xad292db4) at ./Modules/itertoolsmodule.c:413
#21 0x08171b39 in teedataobject_dealloc (tdo=0xad292db4) at ./Modules/itertoolsmodule.c:421
#22 0x0805ed3d in _Py_Dealloc (op=<itertools.tee_dataobject at remote 0xad292db4>) at Objects/object.c:1676
#23 0x08171b16 in teedataobject_clear (tdo=0xad292c94) at ./Modules/itertoolsmodule.c:413
msg148126 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-11-22 16:24
Confirmed on py3k, it doesn't seem to happen with smaller values.
msg148127 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-11-22 16:28
Also, a check for NULL would not hurt in tee_next():

diff -r 1e0e821d2626 Modules/itertoolsmodule.c
--- a/Modules/itertoolsmodule.c	Fri Nov 04 22:17:45 2011 +0100
+++ b/Modules/itertoolsmodule.c	Tue Nov 22 17:24:42 2011 +0100
@@ -475,6 +475,8 @@
 
     if (to->index >= LINKCELLS) {
         link = teedataobject_jumplink(to->dataobj);
+        if (link == NULL)
+            return NULL;
         Py_DECREF(to->dataobj);
         to->dataobj = (teedataobject *)link;
         to->index = 0;
msg172890 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-14 15:47
Here is a patch (tests included). Thank Pyry for report, Victor and Amaury for analysis.

Maybe I picked up the poor names for iterators? May be exhausted and unexhausted would be better? Feel free to correct me.
msg172905 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-14 18:57
Serhiy, I only see a test in your patch, no actual modification to itertools.
msg172916 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-14 20:30
Oh, I worked on it in a different directory. Fortunately I found a temporary copy and I do not have to write all code again. Sorry.
msg174473 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-01 20:43
Please review.
msg178324 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-27 20:43
Ping.
msg178370 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-12-28 07:29
The patch replaces a

 Py_CLEAR(tdo->nextlink)

with a construct that does, basically, something like this several times:

 Py_DECREF(tdo->nextlink)
 tdo->nextlink

which is what leads to the issues that Py_CLEAR is supposed to prevent.  Therefore I think this patch is not correct.
msg178399 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-28 17:26
Good point. Here is updated patch.
msg179466 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-09 16:10
If no one objects I will commit this next week.
msg180568 - (view) Author: Roundup Robot (python-dev) Date: 2013-01-25 11:35
New changeset f7e14a1af609 by Serhiy Storchaka in branch '3.2':
Issue #13454: Fix a crash when deleting an iterator created by itertools.tee()
http://hg.python.org/cpython/rev/f7e14a1af609

New changeset eff2a7346243 by Serhiy Storchaka in branch '3.3':
Issue #13454: Fix a crash when deleting an iterator created by itertools.tee()
http://hg.python.org/cpython/rev/eff2a7346243

New changeset 02d7a127fdfb by Serhiy Storchaka in branch 'default':
Issue #13454: Fix a crash when deleting an iterator created by itertools.tee()
http://hg.python.org/cpython/rev/02d7a127fdfb

New changeset 62f2d3f6015e by Serhiy Storchaka in branch '2.7':
Issue #13454: Fix a crash when deleting an iterator created by itertools.tee()
http://hg.python.org/cpython/rev/62f2d3f6015e
msg180649 - (view) Author: Roundup Robot (python-dev) Date: 2013-01-26 09:56
New changeset 4ee8d38398d4 by Serhiy Storchaka in branch '2.7':
Optimize the test for issue #13454.
http://hg.python.org/cpython/rev/4ee8d38398d4

New changeset d391b2849a51 by Serhiy Storchaka in branch '3.2':
Optimize the test for issue #13454.
http://hg.python.org/cpython/rev/d391b2849a51

New changeset 2258b4d89c9f by Serhiy Storchaka in branch '3.3':
Optimize the test for issue #13454.
http://hg.python.org/cpython/rev/2258b4d89c9f

New changeset 1f57fb5e1e8e by Serhiy Storchaka in branch 'default':
Optimize the test for issue #13454.
http://hg.python.org/cpython/rev/1f57fb5e1e8e
History
Date User Action Args
2013-01-26 09:56:33python-devsetmessages: + msg180649
2013-01-25 11:36:17serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2013-01-25 11:35:11python-devsetnosy: + python-dev
messages: + msg180568
2013-01-09 16:10:11serhiy.storchakasetmessages: + msg179466
2012-12-28 17:26:14serhiy.storchakasetfiles: + itertools_tee_nonrecursive_clear_2.patch

messages: + msg178399
2012-12-28 07:29:22georg.brandlsetnosy: + georg.brandl
messages: + msg178370
2012-12-27 20:48:13serhiy.storchakasetassignee: serhiy.storchaka
2012-12-27 20:43:27serhiy.storchakasetmessages: + msg178324
2012-11-01 20:43:12serhiy.storchakasetkeywords: + needs review

messages: + msg174473
2012-10-15 06:53:42serhiy.storchakasetfiles: + itertools_tee_nonrecursive_clear.patch
2012-10-15 06:52:54serhiy.storchakasetfiles: - itertools_tee_nonrecursive_clear.patch
2012-10-14 20:30:54serhiy.storchakasetfiles: + itertools_tee_nonrecursive_clear.patch

messages: + msg172916
2012-10-14 20:28:22serhiy.storchakasetfiles: - itertools_tee_nonrecursive_clear.patch
2012-10-14 18:57:56pitrousetnosy: + pitrou
messages: + msg172905
2012-10-14 15:47:13serhiy.storchakasetfiles: + itertools_tee_nonrecursive_clear.patch

versions: + Python 3.4
keywords: + patch
nosy: + serhiy.storchaka

messages: + msg172890
stage: test needed -> patch review
2011-11-22 16:28:14amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg148127
2011-11-22 16:24:13ezio.melottisetversions: + Python 3.3
nosy: + rhettinger, ezio.melotti

messages: + msg148126

components: + Extension Modules
stage: test needed
2011-11-22 16:22:32hayposetnosy: + haypo
messages: + msg148125
2011-11-22 16:10:44PyryPcreate