Skip to content

gh-126881: fix finalization of dtoa state #126904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Nov 29, 2024
Merged

gh-126881: fix finalization of dtoa state #126904

merged 4 commits into from
Nov 29, 2024

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Nov 16, 2024 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Nov 16, 2024
Closed
@picnixz picnixz mentioned this pull request Nov 16, 2024
Closed
@kumaraditya303 kumaraditya303 added interpreter-core (Objects, Python, Grammar, and Parser dirs) needs backport to 3.13 bugs and security fixes labels Nov 16, 2024
@devdanzin
Copy link
Contributor

Would a test be helpful here?

skirpichev
skirpichev reviewed Nov 16, 2024
View reviewed changes
@cfbolz
Copy link
Contributor

cfbolz commented Nov 17, 2024

Would a test be helpful here?

Yeah, I was also wondering this. Probably a bit annoying to write a regression test, but I also feel like it would be good to have one.

vstinner
vstinner reviewed Nov 18, 2024
View reviewed changes
@kumaraditya303
Copy link
Contributor Author

Yeah, I was also wondering this. Probably a bit annoying to write a regression test, but I also feel like it would be good to have one.

Regression tests for these don't work well because it relies on GC calling finalizers at late in finalization which isn't reliable, the order or when it will be called is random. As such I avoid adding such tests and manually test these changes.

@kumaraditya303 kumaraditya303 merged commit 762c603 into python:main Nov 29, 2024
45 checks passed
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 29, 2024
@kumaraditya303 @miss-islington
pythongh-126881: fix finalization of dtoa state (pythonGH-126904)
4df6666 
(cherry picked from commit 762c603)

Co-authored-by: Kumar Aditya <[email protected]>
@kumaraditya303 kumaraditya303 deleted the dtoa branch November 29, 2024 09:47
@bedevere-app
Copy link

bedevere-app bot commented Nov 29, 2024

GH-127395 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 Nov 29, 2024
@vstinner
Copy link
Member

Ah, Python 3.12 doesn't have the _PyDtoa_Fini() function.

vstinner pushed a commit that referenced this pull request Nov 29, 2024
@miss-islington @kumaraditya303
[3.13] gh-126881: fix finalization of dtoa state (GH-126904) (#127395)
47b0f8f 
gh-126881: fix finalization of dtoa state (GH-126904)
(cherry picked from commit 762c603)

Co-authored-by: Kumar Aditya <[email protected]>
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
@kumaraditya303 @srinivasreddy
pythongh-126881: fix finalization of dtoa state (python#126904)
e1d9f40
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@kumaraditya303 @ebonnal
pythongh-126881: fix finalization of dtoa state (python#126904)
ee07bab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently left review comments

@skirpichev skirpichev skirpichev left review comments

@vstinner vstinner vstinner approved these changes

Assignees
No one assigned
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kumaraditya303 @devdanzin @cfbolz @vstinner @ericsnowcurrently @skirpichev