Skip to content

gh-131974: Fix usages of locked_deref in ctypes#131975

Merged
vstinner merged 8 commits into
python:mainfrom
ZeroIntensity:gh-131974-ctypes-get-contents-lock
Apr 2, 2025
Merged

gh-131974: Fix usages of locked_deref in ctypes#131975
vstinner merged 8 commits into
python:mainfrom
ZeroIntensity:gh-131974-ctypes-get-contents-lock

Conversation

@ZeroIntensity

@ZeroIntensity ZeroIntensity commented Apr 1, 2025

Copy link
Copy Markdown
Member

We need to hold the lock as long as we plan on using the pointer, not just when we dereference it.

Comment thread Modules/_ctypes/_ctypes.c
Py_ssize_t offset;
PyObject *proto;
void *deref = locked_deref(self);
void *deref = *(void **)self->b_ptr;

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.

You may add a deref_lock_held() function for this operation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Eh, I don't think its necessary. A dereference is easy enough to write.

Comment thread Modules/_ctypes/_ctypes.c
Co-authored-by: Victor Stinner <vstinner@python.org>
Comment thread Modules/_ctypes/_ctypes.c Outdated
Py_ssize_t i;
size_t cur;
for (cur = start, i = 0; i < len; cur += step, i++) {
for (size_t cur = start, Py_ssize_t i = 0; i < len; cur += step, i++) {

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'm very sorry, my suggestion was not valid C code :-(

You should write it differently, example:

    size_t cur = start;
    for (Py_ssize_t i = 0; i < len; cur += step, i++) { 

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No worries, I didn't catch it either. I'll just revert it and leave the formatting as-is.

Comment thread Modules/_ctypes/_ctypes.c Outdated
Comment thread Modules/_ctypes/_ctypes.c
Comment thread Modules/_ctypes/_ctypes.c

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

LGTM

@vstinner vstinner merged commit 6e91d1f into python:main Apr 2, 2025
@vstinner

vstinner commented Apr 2, 2025

Copy link
Copy Markdown
Member

Merged, thank you. I don't think that this change needs to be backported to 3.13.

@ZeroIntensity

Copy link
Copy Markdown
Member Author

It can't; we haven't been backporting ctypes thread safety changes.

@ZeroIntensity ZeroIntensity deleted the gh-131974-ctypes-get-contents-lock branch April 2, 2025 21:03
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.

3 participants