◐ Shell
reader mode source ↗
Skip to content

bpo-31095: fix potential crash during GC.#2974

Merged
methane merged 4 commits into
python:masterfrom
methane:fix/31095
Aug 24, 2017
Merged

bpo-31095: fix potential crash during GC.#2974
methane merged 4 commits into
python:masterfrom
methane:fix/31095

Conversation

@methane

@methane methane commented Aug 1, 2017 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Some tp_dealloc methods of GC types didn't call PyObject_GC_UnTrack().

https://bugs.python.org/issue31095

@methane
bpo-31095: fix potential crash during GC. …
cbce046 
Some tp_dealloc methods of GC types didn't call PyObject_GC_UnTrack().
@methane methane added CLA not signed topic-asyncio type-bug An unexpected behavior, bug, or error labels Aug 1, 2017
@methane methane requested a review from tiran as a code owner August 1, 2017 09:52
@methane methane mentioned this pull request Aug 1, 2017
Merged
serhiy-storchaka
serhiy-storchaka approved these changes Aug 3, 2017
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

LGTM (besides two details).

But this touches delicate things, and I think this PR needs a review of yet one core developer.

benjaminp
benjaminp approved these changes Aug 22, 2017
View reviewed changes

@benjaminp benjaminp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

lgtm. This suggests that we need a systematic way to deal with this problem, though.

vstinner
vstinner previously requested changes Aug 22, 2017
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

I suggest to add a comment explaining why an explicit Untrack is required.

@bedevere-bot

bedevere-bot commented Aug 22, 2017

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I didn't expect the Spanish Inquisition!. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

tiran
tiran approved these changes Aug 22, 2017
View reviewed changes

@tiran tiran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

SSL and elementtree changes LGTM

@methane methane dismissed vstinner’s stale review August 23, 2017 09:38

added required comment

6 hidden items Load more…
@methane methane merged commit a6296d3 into python:master Aug 24, 2017
@methane methane deleted the fix/31095 branch August 24, 2017 05:55
methane added a commit to methane/cpython that referenced this pull request Aug 24, 2017
@methane
[3.6] bpo-31095: fix potential crash during GC (pythonGH-2974) …
c4242b6 
(cherry picked from commit a6296d3)
@bedevere-bot

bedevere-bot commented Aug 24, 2017

Copy link
Copy Markdown

GH-3195 is a backport of this pull request to the 3.6 branch.

methane added a commit to methane/cpython that referenced this pull request Aug 24, 2017
@methane
bpo-31095: fix potential crash during GC (pythonGH-2974) …
2c49cbb 
(cherry picked from commit a6296d3)
GadgetSteve pushed a commit to GadgetSteve/cpython that referenced this pull request Sep 10, 2017
@methane @GadgetSteve
bpo-31095: fix potential crash during GC (pythonGH-2974)
7ff9475
jamadden pushed a commit to zopefoundation/zope.security that referenced this pull request Sep 11, 2017
@jamadden
call PyObject_GC_UnTrack() in tp_dealloc() …
a11c5e2 
see the following sites for details:

* https://bugs.python.org/issue31095
* python/cpython#2974

Fixes #35.
@jamadden jamadden mentioned this pull request Sep 11, 2017
Merged
stephan-hof pushed a commit to zopefoundation/Acquisition that referenced this pull request Sep 11, 2017
call PyObject_GC_UnTrack() in tp_dealloc() …
61bae8d 
    see the following sites for details:
    * https://bugs.python.org/issue31095
    * python/cpython#2974
stephan-hof added a commit to zopefoundation/Acquisition that referenced this pull request Sep 11, 2017
@stephan-hof
call PyObject_GC_UnTrack() in tp_dealloc() …
19ea668 
    see the following sites for details:
    * https://bugs.python.org/issue31095
    * python/cpython#2974
stephan-hof pushed a commit to zopefoundation/Acquisition that referenced this pull request Sep 11, 2017
@stephan-hof
call PyObject_GC_UnTrack() in tp_dealloc() …
11c621e 
    see the following sites for details:
    * https://bugs.python.org/issue31095
    * python/cpython#2974
@miss-islington

miss-islington commented Sep 13, 2017

Copy link
Copy Markdown
Contributor

Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington

miss-islington commented Sep 13, 2017

Copy link
Copy Markdown
Contributor

Sorry, @methane, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.

@vstinner

vstinner commented Sep 13, 2017

Copy link
Copy Markdown
Member

The commit 4cde4bd is the backport to Python 2.7.

y-fujisaki2 added a commit to y-fujisaki2/zope.interface that referenced this pull request Sep 19, 2017
@y-fujisaki2
Fix process of reference count during GC …
6ba212b 
call PyObject_GC_UnTrack() in tp_dealloc()
see the following sites for details:
 * https://bugs.python.org/issue31095
 * python/cpython#2974
@y-fujisaki2 y-fujisaki2 mentioned this pull request Sep 19, 2017
Merged
y-fujisaki2 added a commit to y-fujisaki2/zope.container that referenced this pull request Sep 21, 2017
@y-fujisaki2
Fix process of reference count during GC …
40aa0a9 
call PyObject_GC_UnTrack() in tp_dealloc()
see the following sites for details:
* https://bugs.python.org/issue31095
* python/cpython#2974
y-fujisaki2 added a commit to y-fujisaki2/zope.i18nmessageid that referenced this pull request Sep 21, 2017
@y-fujisaki2
Fix process of reference count during GC …
0faafd2 
call PyObject_GC_UnTrack() in tp_dealloc()
see the following sites for details:
* https://bugs.python.org/issue31095
* python/cpython#2974
y-fujisaki2 added a commit to y-fujisaki2/zope.i18nmessageid that referenced this pull request Sep 26, 2017
@y-fujisaki2
Fix process of reference count during GC …
83c5b8f 
    call PyObject_GC_UnTrack() in tp_dealloc()
    see the following sites for details:
    * https://bugs.python.org/issue31095
    * python/cpython#2974
y-fujisaki2 added a commit to y-fujisaki2/zope.container that referenced this pull request Sep 26, 2017
@y-fujisaki2
Fix process of reference count during GC …
5978d8b 
    call PyObject_GC_UnTrack() in tp_dealloc()
    see the following sites for details:
    * https://bugs.python.org/issue31095
    * python/cpython#2974
larryhastings pushed a commit that referenced this pull request Sep 26, 2017
@methane @larryhastings
bpo-31095: fix potential crash during GC (GH-2974) (#3196) …
0fcc033 
(cherry picked from commit a6296d3)
hannosch pushed a commit to zopefoundation/Acquisition that referenced this pull request Dec 1, 2017
@hannosch
call PyObject_GC_UnTrack() in tp_dealloc() …
95a045d 
    see the following sites for details:
    * https://bugs.python.org/issue31095
    * python/cpython#2974
@neonene neonene mentioned this pull request Mar 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@benjaminp benjaminp benjaminp approved these changes

@tiran tiran tiran approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

No one assigned

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@methane @bedevere-bot @miss-islington @vstinner @benjaminp @tiran @serhiy-storchaka @Mariatta @the-knights-who-say-ni