Skip to content
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.

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

gh-99952: [ctypes] fix refcount issues in from_param() result. #100169

Merged
merged 8 commits into from
Jan 26, 2023

Conversation

ynkdir
Copy link
Contributor

@ynkdir ynkdir commented Dec 11, 2022

Fixes a reference counting issue with ctypes.Structure when a from_param() method call is used and the structure size is larger than a C pointer sizeof(void*).

This problem existed for a very long time, but became more apparent in 3.8+ by change likely due to garbage collection cleanup timing changes.

@netlify
Copy link

netlify bot commented Dec 11, 2022

Deploy Preview for python-cpython-preview canceled.

Name Link
🔨 Latest commit ef884f1
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/6395871f802ebd000860138d

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Dec 11, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@ynkdir
Copy link
Contributor Author

ynkdir commented Dec 12, 2022

related changelog
bpo-35947: Update windows to the current version of libffi #11797
bpo-37140: Fix StructUnionType_paramfunc() #15612

@junkmd
Copy link

junkmd commented Dec 13, 2022

🚀

@FFY00
Copy link
Member

FFY00 commented Dec 19, 2022

I am not comfortable enough judging if this is the best/correct fix, but it seems like something that @vstinner might enjoy having a look at.

@FFY00 FFY00 removed their request for review December 19, 2022 19:56
@junkmd
Copy link

junkmd commented Dec 24, 2022

🎄 🎁 🧑‍🎄 🤶
Just remind.

@dpy013
Copy link

dpy013 commented Jan 12, 2023

hello all
Is there an update on the current pr?

@junkmd
Copy link

junkmd commented Jan 14, 2023

The absence of updates may be because the expert-ctypes label is not attached to this PR, even though it is attached to issue gh-99952.

Looking at the PRs with the expert-ctypes label that have been merged, I think the other core developers involved to ctypes are @zware, @AA-Turner and @gpshead.

Triage and labeling to PRs seems to be done by @AlexWaygood.

I hope that they are aware of this ping and will respond.

@zware
Copy link
Member

zware commented Jan 16, 2023

This looks reasonable as far as I can tell, but unfortunately my expertise with ctypes only extends as far as removing ancient copies of libffi from the repository :)

@gpshead gpshead self-assigned this Jan 26, 2023
@gpshead gpshead added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes type-bug An unexpected behavior, bug, or error labels Jan 26, 2023
@miss-islington
Copy link
Contributor

Thanks @ynkdir for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@gpshead
Copy link
Member

gpshead commented Jan 26, 2023

Thanks everyone, this kind of bug can be particularly challenging to diagnose!

@miss-islington
Copy link
Contributor

Sorry, @ynkdir and @gpshead, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker dfad678d7024ab86d265d84ed45999e031a03691 3.11

@miss-islington
Copy link
Contributor

Sorry @ynkdir and @gpshead, I had trouble checking out the 3.10 backport branch.
Please retry by removing and re-adding the "needs backport to 3.10" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker dfad678d7024ab86d265d84ed45999e031a03691 3.10

gpshead pushed a commit to gpshead/cpython that referenced this pull request Jan 26, 2023
…esult. (pythonGH-100169)

Fixes a reference counting issue with `ctypes.Structure` when a `from_param()` method call is used and the structure size is larger than a C pointer `sizeof(void*)`.

This problem existed for a very long time, but became more apparent in 3.8+ by change likely due to garbage collection cleanup timing changes..
(cherry picked from commit dfad678)

Co-authored-by: Yukihiro Nakadaira <[email protected]>
gpshead added a commit that referenced this pull request Jan 26, 2023
… result (#101339)

[3.11] gh-99952: [ctypes] fix refcount issues in from_param() result. (GH-100169)

Fixes a reference counting issue with `ctypes.Structure` when a `from_param()` method call is used and the structure size is larger than a C pointer `sizeof(void*)`.

This problem existed for a very long time, but became more apparent in 3.8+ by change likely due to garbage collection cleanup timing changes..
(cherry picked from commit dfad678)

Co-authored-by: Yukihiro Nakadaira <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 26, 2023
…aram() result (pythonGH-101339)

[3.11] pythongh-99952: [ctypes] fix refcount issues in from_param() result. (pythonGH-100169)

Fixes a reference counting issue with `ctypes.Structure` when a `from_param()` method call is used and the structure size is larger than a C pointer `sizeof(void*)`.

This problem existed for a very long time, but became more apparent in 3.8+ by change likely due to garbage collection cleanup timing changes..
(cherry picked from commit dfad678)

(cherry picked from commit fa7c37a)

Co-authored-by: Gregory P. Smith <[email protected]>
Co-authored-by: Yukihiro Nakadaira <[email protected]>
@ynkdir
Copy link
Contributor Author

ynkdir commented Jan 26, 2023

Thank you.

mdboom pushed a commit to mdboom/cpython that referenced this pull request Jan 31, 2023
…ython#100169)

Fixes a reference counting issue with `ctypes.Structure` when a `from_param()` method call is used and the structure size is larger than a C pointer `sizeof(void*)`.

This problem existed for a very long time, but became more apparent in 3.8+ by change likely due to garbage collection cleanup timing changes.
gpshead added a commit that referenced this pull request Feb 4, 2023
…param() result (GH-101339) (#101340)

[3.11] gh-99952: [ctypes] fix refcount issues in from_param() result. (GH-100169)

Fixes a reference counting issue with `ctypes.Structure` when a `from_param()` method call is used and the structure size is larger than a C pointer `sizeof(void*)`.

This problem existed for a very long time, but became more apparent in 3.8+ by change likely due to garbage collection cleanup timing changes..
(cherry picked from commit dfad678)

(cherry picked from commit fa7c37a)

Co-authored-by: Gregory P. Smith <[email protected]>
Co-authored-by: Yukihiro Nakadaira <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
@zware zware removed needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-ctypes type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants