Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

bpo-38031: Fix a possible assertion failure in _io.FileIO()#15688

Merged
iritkatriel merged 12 commits into
python:mainpython/cpython:mainfrom
ZackerySpytz:bpo-38031-_io-FileIO-opener-crashZackerySpytz/cpython:bpo-38031-_io-FileIO-opener-crashCopy head branch name to clipboard
Nov 25, 2022
Merged

bpo-38031: Fix a possible assertion failure in _io.FileIO()#15688
iritkatriel merged 12 commits into
python:mainpython/cpython:mainfrom
ZackerySpytz:bpo-38031-_io-FileIO-opener-crashZackerySpytz/cpython:bpo-38031-_io-FileIO-opener-crashCopy head branch name to clipboard

Conversation

@ZackerySpytz

@ZackerySpytz ZackerySpytz commented Sep 5, 2019

Copy link
Copy Markdown
Contributor

Use PyErr_Fetch() / _PyErr_ChainExceptions() when calling
internal_close() on error. The opener may return an invalid
fd, which will cause the close() call in internal_close() to fail.

https://bugs.python.org/issue38031

Use PyErr_Fetch() / _PyErr_ChainExceptions() when calling
internal_close() on error.  The opener may return an invalid
fd, which will cause the close() call in internal_close() to fail.
@aeros aeros added the type-bug An unexpected behavior, bug, or error label Sep 5, 2019

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

Thanks for the PR @ZackerySpytz.

The macOS failures in Azure may be related to https://bugs.python.org/issue37245 and not related to this PR. (Build log: https://dev.azure.com/Python/cpython/_build/results?buildId=49868&view=logs&j=18d1a34d-6940-5fc1-f55b-405e2fba32b1)

2 tests failed: test_functools test_multiprocessing_spawn

Both of the above tests timed out.

Comment thread Lib/test/test_io.py Outdated
Comment thread Lib/test/test_io.py Outdated
Comment thread Lib/test/test_io.py Outdated
@vstinner

vstinner commented Sep 6, 2019

Copy link
Copy Markdown
Member

cc @serhiy-storchaka @pitrou

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

Mostly LGTH. Just address Victor's comment.

@csabella

csabella commented Feb 4, 2020

Copy link
Copy Markdown
Contributor

@ZackerySpytz, please address the code review. Thanks!

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

Converted @vstinner's comment into suggestions.

Comment thread Lib/test/test_io.py Outdated
Comment thread Lib/test/test_io.py Outdated
Comment thread Lib/test/test_io.py Outdated
Comment thread Lib/test/test_io.py Outdated
Comment thread Lib/test/test_io.py Outdated
Comment thread Lib/test/test_io.py
Comment thread Lib/test/test_io.py Outdated
Comment thread Lib/test/test_io.py
Comment thread Lib/test/test_io.py
Comment thread Lib/test/test_io.py Outdated

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

LGTM. Thanks for updating the PR @iritkatriel ;-)

@iritkatriel iritkatriel added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Nov 24, 2022
@iritkatriel iritkatriel self-assigned this Nov 24, 2022
Comment thread Lib/test/test_io.py Outdated
@iritkatriel iritkatriel merged commit d386115 into python:main Nov 25, 2022
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @ZackerySpytz for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Morty Proxy This is a proxified and sanitized view of the page, visit original site.