Skip to content

Conversation

@ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented Jul 14, 2018

@serhiy-storchaka
Copy link
Member

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);
    }

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.

@@ -0,0 +1,2 @@
In :meth:`io.IOBase.close`, ensure that ``_PyObject_SetAttrId()`` is not
Copy link
Member

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

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

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

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

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
… an exception set (pythonGH-8282)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <[email protected]>
@miss-islington
Copy link
Contributor

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

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
…d with an exception set (pythonGH-8282).

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <[email protected]>
@bedevere-bot
Copy link

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
… an exception set (GH-8282)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <[email protected]>
serhiy-storchaka added a commit that referenced this pull request Jul 17, 2018
… an exception set (GH-8282). (GH-8312)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <[email protected]>
@vstinner
Copy link
Member

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
…d with an exception set (pythonGH-8282). (pythonGH-8312)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <[email protected]>.
(cherry picked from commit cc13016)

Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this pull request Jul 17, 2018
…an exception set (GH-8282). (GH-8312) (GH-8314)

(cherry picked from commit 28f0736)

Co-authored-by: Zackery Spytz <[email protected]>.
(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

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants