classification
Title: Defer DECREFs until enum object is in a consistent state for re-entrancy
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: rhettinger, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2016-03-06 10:23 by rhettinger, last changed 2017-09-25 09:16 by rhettinger. This issue is now closed.

Files
File name Uploaded Description Edit
enumerate_defer_decref.diff rhettinger, 2016-03-06 10:23 review
Pull Requests
URL Status Linked Edit
PR 3747 merged rhettinger, 2017-09-25 08:56
Messages (6)
msg261246 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-06 11:45
I don't see possible bug here.

The first item of the tuple is always int or None. It's decrefing don't trigger executing user code.

The second item of the tuple is decrefed when the refcount of the tuple is 2 and we own all references. The code triggered by decrefing the old item can't use the tuple (since its refcount is not 1 now), nor free the new index or new item (since we take the ownership of them).

I think this patch is not needed.
msg263284 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-12 23:08
Raymond: Do you have an example to trigger the issue?
msg263289 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-04-13 00:34
> status: pending -> open

Oh, this change was not intended. I don't know how it occurred.
msg263301 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-04-13 03:18
Deferring decrefs as late as possible is a good practice, reducing the risk of bugs being introduced later.  There have been other places where there were bugs that arose due to premature decreffing.

I wrote the original code for enumerate, am the primary maintainer, and can make improvements to code organization as needed.  Unless you think this change is broken, it should stay in.
msg263307 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-04-13 05:08
This change is not broken. It is just redundant.

If you are still inclined to apply this changes, I would suggest to factor out common code in enum_next_long and enum_next.

> Oh, this change was not intended. I don't know how it occurred.

Any reply changes the status from pending to open.
msg302933 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-25 09:15
New changeset 8110dbd470f3daa4de58dda66d360e3c26d3b94f by Raymond Hettinger in branch 'master':
bpo-26491 Defer DECREFs until enumobject is in a consistent state (#3747)
https://github.com/python/cpython/commit/8110dbd470f3daa4de58dda66d360e3c26d3b94f
History
Date User Action Args
2017-09-25 09:16:43rhettingersetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.7, - Python 2.7, Python 3.5, Python 3.6
2017-09-25 09:15:57rhettingersetmessages: + msg302933
2017-09-25 08:56:31rhettingersetpull_requests: + pull_request3734
2017-06-16 06:02:27rhettingersetstatus: pending -> open
2017-06-15 18:35:32serhiy.storchakasetstatus: open -> pending
2017-01-23 18:42:50serhiy.storchakasetstatus: pending -> open
assignee: rhettinger
2016-10-04 18:39:29serhiy.storchakasetstatus: open -> pending
2016-04-13 05:08:09serhiy.storchakasetmessages: + msg263307
2016-04-13 03:18:33rhettingersetstatus: pending -> open

messages: + msg263301
2016-04-13 00:34:20vstinnersetstatus: open -> pending

messages: + msg263289
2016-04-12 23:08:02vstinnersetstatus: pending -> open
nosy: + vstinner
messages: + msg263284

2016-04-12 06:43:40serhiy.storchakasetstatus: open -> pending
2016-03-12 07:56:47serhiy.storchakasetassignee: serhiy.storchaka -> (no value)
2016-03-06 11:45:26serhiy.storchakasetmessages: + msg261246
2016-03-06 10:23:02rhettingercreate