Skip to content

bpo-34068: _io__IOBase_close_impl could call _PyObject_SetAttrId with an exception set#8282

Merged
serhiy-storchaka merged 2 commits intopython:masterfrom
ZackerySpytz:bpo-34068
Jul 17, 2018
Merged

bpo-34068: _io__IOBase_close_impl could call _PyObject_SetAttrId with an exception set#8282
serhiy-storchaka merged 2 commits intopython:masterfrom
ZackerySpytz:bpo-34068

Conversation

@ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented Jul 14, 2018 •
edited by bedevere-bot
Loading

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Jul 14, 2018

The code could be simpler:

    PyErr_Fetch(&exc, &val, &tb);
    rc = _PyObject_SetAttrId(self, &PyId___IOBase_closed, Py_True);
    _PyErr_ChainExceptions(exc, val, tb);
    if (rc < 0) {
        Py_CLEAR(res);
    }

vstinner
vstinner approved these changes Jul 16, 2018
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM, but I have a minor request on the NEWS entry.

Misc/NEWS.d/next/Core and Builtins/2018-07-14-08-58-46.bpo-34068.9xfM55.rst Outdated
@@ -0,0 +1,2 @@
In :meth:`io.IOBase.close`, ensure that ``_PyObject_SetAttrId()`` is not
Copy link
Member

@vstinner vstinner Jul 16, 2018

Choose a reason for hiding this comment

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

In the changelog, you should try to avoid to mention low-level C internals. You can say something like: "In io.IOBase.close, ensure that the closed attribute is not set to True with a live exception. (...)".

@serhiy-storchaka serhiy-storchaka merged commit 28f0736 into python:master Jul 17, 2018
@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.6 labels Jul 17, 2018
@miss-islington
Copy link
Contributor

miss-islington commented Jul 17, 2018

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Jul 17, 2018

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Jul 17, 2018

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

bedevere-bot commented Jul 17, 2018

GH-8310 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 17, 2018
@ZackerySpytz @miss-islington
bpo-34068: _io__IOBase_close_impl could call _PyObject_SetAttrId with…
9bd4fb5 
… an exception set (pythonGH-8282)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@miss-islington
Copy link
Contributor

miss-islington commented Jul 17, 2018

Sorry, @ZackerySpytz and @serhiy-storchaka, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 28f07364f066792ceee93231dbb80ae8ad98b2bb 3.6

@miss-islington
Copy link
Contributor

miss-islington commented Jul 17, 2018

Sorry, @ZackerySpytz and @serhiy-storchaka, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 28f07364f066792ceee93231dbb80ae8ad98b2bb 2.7

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Jul 17, 2018
@ZackerySpytz @serhiy-storchaka
[3.6] bpo-34068: _io__IOBase_close_impl could call _PyObject_SetAttrI…
f780462 
…d with an exception set (pythonGH-8282).

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@bedevere-bot
Copy link

bedevere-bot commented Jul 17, 2018

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

miss-islington added a commit that referenced this pull request Jul 17, 2018
@miss-islington @ZackerySpytz
bpo-34068: _io__IOBase_close_impl could call _PyObject_SetAttrId with…
6020d98 
… an exception set (GH-8282)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
serhiy-storchaka added a commit that referenced this pull request Jul 17, 2018
@serhiy-storchaka @ZackerySpytz
bpo-34068: _io__IOBase_close_impl could call _PyObject_SetAttrId with…
cc13016 
… an exception set (GH-8282). (GH-8312)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@vstinner
Copy link
Member

vstinner commented Jul 17, 2018

Tweak the news entry.

Thanks. It now LGTM ;-) Thanks for the fix @ZackerySpytz!

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jul 17, 2018
@serhiy-storchaka
[2.7] bpo-34068: _io__IOBase_close_impl could call _PyObject_SetAttrI…
4744a1a 
…d with an exception set (pythonGH-8282). (pythonGH-8312)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>.
(cherry picked from commit cc13016)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit that referenced this pull request Jul 17, 2018
@serhiy-storchaka
[2.7] bpo-34068: iobase_close could call PyObject_SetAttrString with …
fc153d1 
…an exception set (GH-8282). (GH-8312) (GH-8314)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>.
(cherry picked from commit cc13016)
@ZackerySpytz ZackerySpytz deleted the bpo-34068 branch August 29, 2018 22:12
@serhiy-storchaka serhiy-storchaka removed their assignment Sep 22, 2018
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

@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.

6 participants

@ZackerySpytz @serhiy-storchaka @miss-islington @bedevere-bot @vstinner @the-knights-who-say-ni