Skip to content

gh-127667: fix more reference leaks in hashlib#127668

Merged
picnixz merged 18 commits into
python:mainfrom
picnixz:fix/hashlib/error-branches-127667
Mar 3, 2025
Merged

gh-127667: fix more reference leaks in hashlib#127668
picnixz merged 18 commits into
python:mainfrom
picnixz:fix/hashlib/error-branches-127667

Conversation

@picnixz

@picnixz picnixz commented Dec 6, 2024

Copy link
Copy Markdown
Member

I took the liberty of changing how exceptions are being set in hashlib because _setException(PyExc_ValueError, NULL); is honestly confusing (namely) that "NULL" stands for a default message when the OpenSSL error message cannot be retrieved). (this will be in a follow-up PR)

In this PR we:

  • release allocated resources in error-branches where they were previously leaking;
  • consistently collapse _setException() + return into one statement when possible; and
  • suppress unused return values for _setException().

@picnixz picnixz added skip news needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Dec 6, 2024
@picnixz picnixz marked this pull request as draft December 6, 2024 09:30
@picnixz picnixz marked this pull request as ready for review December 6, 2024 10:41
@picnixz picnixz requested a review from gpshead December 6, 2024 16:24
@picnixz picnixz added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Dec 6, 2024
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 1b7c775 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Dec 6, 2024
@picnixz picnixz added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 6, 2024
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 1b7c775 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 6, 2024
@erlend-aasland

Copy link
Copy Markdown
Contributor

I'd prefer if this could be broken in two PRs: one for the refactoring, and one for the error branch fixes.

@picnixz

picnixz commented Jan 4, 2025

Copy link
Copy Markdown
Member Author

I'd prefer if this could be broken in two PRs: one for the refactoring, and one for the error branch fixes.

Sure! I'll split them into two tomorrow.

@picnixz picnixz marked this pull request as draft January 5, 2025 08:35
@picnixz picnixz force-pushed the fix/hashlib/error-branches-127667 branch from f2a0f6f to 9e34c0e Compare January 5, 2025 08:44
@picnixz picnixz marked this pull request as ready for review January 5, 2025 08:47
@picnixz picnixz marked this pull request as draft March 2, 2025 12:48
@picnixz picnixz marked this pull request as ready for review March 2, 2025 12:52
@picnixz picnixz requested a review from tiran as a code owner March 2, 2025 12:52
@picnixz picnixz requested a review from encukou March 2, 2025 12:52
@picnixz picnixz changed the title gh-127667: improve hashlib error-branches gh-127667: fix more reference leaks in hashlib Mar 2, 2025
@picnixz picnixz self-assigned this Mar 2, 2025
@picnixz picnixz merged commit 0978465 into python:main Mar 3, 2025
@miss-islington-app

Copy link
Copy Markdown

Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

@picnixz picnixz deleted the fix/hashlib/error-branches-127667 branch March 3, 2025 08:20
@miss-islington-app

Copy link
Copy Markdown

Sorry, @picnixz, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 097846502b7f33cb327d512e2a396acf4f4de46e 3.13

@miss-islington-app

Copy link
Copy Markdown

Sorry, @picnixz, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 097846502b7f33cb327d512e2a396acf4f4de46e 3.12

@picnixz

picnixz commented Mar 3, 2025

Copy link
Copy Markdown
Member Author

Arf, this one won't backport cleanly because it contains some references to the UBSan failures fixes.

picnixz added a commit to picnixz/cpython that referenced this pull request Mar 3, 2025
- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.
@bedevere-app

bedevere-app Bot commented Mar 3, 2025

Copy link
Copy Markdown

GH-130783 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Mar 3, 2025
@bedevere-app

bedevere-app Bot commented Mar 3, 2025

Copy link
Copy Markdown

GH-130783 is a backport of this pull request to the 3.12 branch.

1 similar comment
@bedevere-app

bedevere-app Bot commented Mar 3, 2025

Copy link
Copy Markdown

GH-130783 is a backport of this pull request to the 3.12 branch.

picnixz added a commit to picnixz/cpython that referenced this pull request Mar 3, 2025
- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.
@bedevere-app

bedevere-app Bot commented Mar 3, 2025

Copy link
Copy Markdown

GH-130784 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 Mar 3, 2025
picnixz added a commit that referenced this pull request Mar 3, 2025
gh-127667: fix memory leaks in `hashlib` (GH-127668)

- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.

(cherry-picked from commit 0978465)
picnixz added a commit that referenced this pull request Mar 3, 2025
gh-127667: fix memory leaks in `hashlib` (GH-127668)

- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.

(cherry-picked from commit 0978465)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants