FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-39091: Fix segfault when Exception constructor returns non-exception for gen.throw by coolreader18 · Pull Request #17658 · python/cpython · GitHub

/ cpython Public

bpo-39091: Fix segfault when Exception constructor returns non-exception for gen.throw - #17658

Merged
benjaminp merged 8 commits into
python:mainfrom
coolreader18:gen-throw-bpo-39091
Aug 3, 2021
ConversationCommits (8)ChecksFiles changed
Merged

bpo-39091: Fix segfault when Exception constructor returns non-exception for gen.throw#17658
benjaminp merged 8 commits into
python:mainfrom
coolreader18:gen-throw-bpo-39091

Conversation

coolreader18 commented Dec 19, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

I added a type check in _PyErr_CreateException as @serhiy-storchaka suggested.

I'm not sure if I should replace the check in do_raise in ceval.c with a call to _PyErr_CreateException somehow, as it's effectively the same functionality?

https://bugs.python.org/issue39091

the-knights-who-say-ni commented Dec 19, 2019

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@coolreader18

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

coolreader18 changed the title bpo-39091: Fix segfault when Exception constructor returns non-exception for gen.throw (and probably some other places) bpo-39091: Fix segfault when Exception constructor returns non-exception for gen.throw Dec 19, 2019
eric-wieser reviewed Dec 19, 2019
View reviewed changes
Comment thread Lib/test/test_generators.py Outdated
coolreader18 force-pushed the gen-throw-bpo-39091 branch 2 times, most recently from ad65177 to 08e42d6 Compare December 19, 2019 23:25
eric-wieser reviewed Dec 20, 2019
View reviewed changes
Comment thread Lib/test/test_generators.py Outdated
serhiy-storchaka reviewed Dec 21, 2019
View reviewed changes
Comment thread Python/errors.c Outdated
Comment thread Python/errors.c Outdated
Comment thread Python/errors.c Outdated

serhiy-storchaka commented Dec 21, 2019

Copy link
Copy Markdown
Member

do_raise has a complex code. Unless it contains similar bug I suggest to left refactoring to other issue.

ZackerySpytz reviewed Dec 23, 2019
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2019-12-21-14-18-32.bpo-39091.dOexgQ.rst Outdated
Comment thread Misc/ACKS Outdated

coolreader18 commented Jan 15, 2020

Copy link
Copy Markdown
Contributor Author

Any updates on this?

corona10 reviewed Feb 3, 2020
View reviewed changes

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@serhiy-storchaka @vstinner
This PR is waiting for review for a long time.
Can you please take a look?

corona10 requested a review from vstinner February 3, 2020 14:02

codecov Bot commented Feb 16, 2020

Copy link
Copy Markdown

Codecov Report

Merging #17658 into master will increase coverage by 1.08%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #17658       +/-   ##
===========================================
+ Coverage   82.11%   83.20%    +1.08%     
===========================================
  Files        1956     1571      -385     
  Lines      589364   414752   -174612     
  Branches    44457    44457               
===========================================
- Hits       483963   345084   -138879     
+ Misses      95748    60021    -35727     
+ Partials     9653     9647        -6
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
Lib/test/test_asyncio/test_base_events.py 91.84% <0.00%> (-3.30%) ⬇️
... and 434 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c33bdbb...95254b4. Read the comment docs.

bl-ue commented Jun 17, 2021

Copy link
Copy Markdown

ping @serhiy-storchaka @vstinner this PR needs to be merged.

benjaminp merged commit 83ca46b into python:main Aug 3, 2021

bedevere-bot commented Aug 3, 2021

Copy link
Copy Markdown

@benjaminp: Please replace # with GH- in the commit message next time. Thanks!

miss-islington commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Thanks @coolreader18 for the PR, and @benjaminp for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.8, 3.9.
🐍🍒⛏🤖

bedevere-bot commented Aug 3, 2021

Copy link
Copy Markdown

GH-27572 is a backport of this pull request to the 3.10 branch.

bedevere-bot commented Aug 3, 2021

Copy link
Copy Markdown

GH-27573 is a backport of this pull request to the 3.9 branch.

miss-islington commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Sorry, @coolreader18 and @benjaminp, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 83ca46b7784b7357d82ec47b33295e09ed7380cb 3.8

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 3, 2021
closes bpo-39091: Fix segfault when Exception constructor returns non…
bb058c9 
…-exception for gen.throw. (pythonGH-17658)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
(cherry picked from commit 83ca46b)

Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 3, 2021
closes bpo-39091: Fix segfault when Exception constructor returns non…
f99652e 
…-exception for gen.throw. (pythonGH-17658)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
(cherry picked from commit 83ca46b)

Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
ambv pushed a commit that referenced this pull request Aug 3, 2021
bpo-39091: Fix segfault when Exception constructor returns non-except…
8ce7f2f 
…ion for gen.throw. (GH-17658) (GH-27572)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
(cherry picked from commit 83ca46b)

Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
ambv pushed a commit that referenced this pull request Aug 3, 2021
bpo-39091: Fix segfault when Exception constructor returns non-except…
0b551db 
…ion for gen.throw. (GH-17658) (GH-27573)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
(cherry picked from commit 83ca46b)

Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>

vstinner commented Aug 6, 2021

Copy link
Copy Markdown
Member

Thanks for the fix!

nocturn9x commented Aug 25, 2021

Copy link
Copy Markdown

Took 2 years but it's finally getting fixed! 😂

vstinner commented Aug 30, 2021

Copy link
Copy Markdown
Member

Better late than never.

coolreader18 deleted the gen-throw-bpo-39091 branch August 31, 2021 02:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

corona10 corona10 left review comments
serhiy-storchaka Awaiting requested review from serhiy-storchaka
vstinner Awaiting requested review from vstinner
+2 more reviewers
eric-wieser eric-wieser left review comments
ZackerySpytz ZackerySpytz left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

benjaminp

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL