Skip to content

gh-126881: fix finalization of dtoa state#126904

Merged
kumaraditya303 merged 4 commits intopython:mainfrom
kumaraditya303:dtoa
Nov 29, 2024
Merged

gh-126881: fix finalization of dtoa state#126904
kumaraditya303 merged 4 commits intopython:mainfrom
kumaraditya303:dtoa

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
Member

devdanzin commented Nov 16, 2024

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
ericsnowcurrently
ericsnowcurrently reviewed Nov 19, 2024
View reviewed changes
@kumaraditya303
Copy link
Contributor Author

kumaraditya303 commented Nov 29, 2024

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
@miss-islington-app
Copy link

miss-islington-app bot commented Nov 29, 2024

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

vstinner commented Nov 29, 2024

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