Skip to content

gh-132063: ProcessPoolExecutor swallows falsy Exceptions#132129

Merged
picnixz merged 11 commits into
python:mainfrom
YvesDup:ProcessPoolExecutor-swallows-flasy-exception
Apr 8, 2025
Merged

gh-132063: ProcessPoolExecutor swallows falsy Exceptions#132129
picnixz merged 11 commits into
python:mainfrom
YvesDup:ProcessPoolExecutor-swallows-flasy-exception

Conversation

@YvesDup

@YvesDup YvesDup commented Apr 5, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

ProcessPoolExecutor and ThreadPoolExecutor swallows falsy exception.

Fix tests about exception variable in 2 files of /Lib/concurrent/futures

  • _base.py
  • process.py

Changes 2 tests as below:

Replace if exception: with if exception is not None:

@bedevere-app bedevere-app Bot mentioned this pull request Apr 5, 2025
Closed
ZeroIntensity
ZeroIntensity reviewed Apr 5, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2025-04-05-15-05-09.gh-issue-132063.KHnslU.rst Outdated
Comment thread Lib/test/test_concurrent_futures/test_process_pool.py Outdated
Comment thread Lib/test/test_concurrent_futures/test_process_pool.py Outdated
Comment thread Lib/test/test_concurrent_futures/test_process_pool.py Outdated
Comment thread Lib/test/test_concurrent_futures/test_thread_pool.py Outdated
@python-cla-bot

python-cla-bot Bot commented Apr 6, 2025

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

picnixz
picnixz reviewed Apr 6, 2025
View reviewed changes

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

I never sent my review.

Comment thread Misc/NEWS.d/next/Library/2025-04-05-15-05-09.gh-issue-132063.KHnslU.rst Outdated
Comment thread Lib/test/test_concurrent_futures/test_process_pool.py Outdated
@YvesDup

YvesDup commented Apr 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

After more tests, I found that __cause__, __context__ attributes in falsey exceptions are ignored.

cpython/Lib/traceback.py

Line 1123 in 255eb37

if (e and e.__cause__ is not None

cpython/Lib/traceback.py

Line 1144 in 255eb37

if (e and e.__context__ is not None

And falsey BaseExceptionGroup exception is ignored too.

cpython/Lib/traceback.py

Line 1159 in 255eb37

if e and isinstance(e, BaseExceptionGroup):

Should we open a new issue with these bugs or integrate them to this PR ?

EDIT: please ignore this comment

picnixz
picnixz reviewed Apr 8, 2025
View reviewed changes
Comment thread Lib/test/test_concurrent_futures/executor.py
Comment thread Misc/NEWS.d/next/Library/2025-04-05-15-05-09.gh-issue-132063.KHnslU.rst Outdated
YvesDup and others added 3 commits April 8, 2025 14:18
@YvesDup @picnixz
Update news
e450739 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@YvesDup @picnixz
Add a blank line
ea66f0b 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@YvesDup
Merge branch 'main' into ProcessPoolExecutor-swallows-flasy-exception
5a4e477
picnixz
picnixz approved these changes Apr 8, 2025
View reviewed changes

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

Some nits and LGTM.

Comment thread Lib/test/test_concurrent_futures/executor.py
Comment thread Lib/test/test_concurrent_futures/executor.py Outdated
Comment thread Lib/test/test_concurrent_futures/executor.py Outdated
picnixz
picnixz approved these changes Apr 8, 2025
View reviewed changes
Comment thread Lib/test/test_concurrent_futures/executor.py

msg = 'boolbool'
with self.assertRaisesRegex(FalseyBoolException, msg):
self.executor.submit(raiser, FalseyBoolException, msg).result()

@picnixz picnixz Apr 8, 2025

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.

This is for a possible follow-up but this kind of call makes me wonder whether the base class shouldn't have some def submit_and_fetch(self) method that does this.

@YvesDup YvesDup Apr 8, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this method return directy the result ?

@YvesDup YvesDup Apr 8, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is a follow-up with the TracebackExceptionclass which swallows __cause__ and __context__ content of falsey exceptions.
I will post a new issue about this.

@picnixz
picnixz enabled auto-merge (squash) April 8, 2025 14:55
@picnixz picnixz added the needs backport to 3.13 bugs and security fixes label Apr 8, 2025
@picnixz
picnixz merged commit 933c665 into python:main Apr 8, 2025
@miss-islington-app

miss-islington-app Bot commented Apr 8, 2025

Copy link
Copy Markdown

Thanks @YvesDup for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 8, 2025
@YvesDup @miss-islington
pythongh-132063: ensure that ProcessPoolExecutor does not swallow f…
1503976 
…alsey exceptions (pythonGH-132129)

(cherry picked from commit 933c665)

Co-authored-by: Duprat <yduprat@gmail.com>
@bedevere-app

bedevere-app Bot commented Apr 8, 2025

Copy link
Copy Markdown

GH-132275 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Apr 8, 2025
picnixz pushed a commit that referenced this pull request Apr 8, 2025
@miss-islington @YvesDup
[3.13] gh-132063: ensure that ProcessPoolExecutor does not swallow …
8f6b0af 
…falsey exceptions (GH-132129) (#132275)

gh-132063: ensure that `ProcessPoolExecutor` does not swallow falsey exceptions (GH-132129)
(cherry picked from commit 933c665)

Co-authored-by: Duprat <yduprat@gmail.com>
@YvesDup YvesDup mentioned this pull request Apr 9, 2025
Closed
@YvesDup
YvesDup deleted the ProcessPoolExecutor-swallows-flasy-exception branch May 16, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeroIntensity ZeroIntensity ZeroIntensity left review comments
@picnixz picnixz picnixz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@YvesDup @picnixz @ZeroIntensity