-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
gh-131974: Fix usages of locked_deref
in ctypes
#131975
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-131974: Fix usages of locked_deref
in ctypes
#131975
Conversation
{ | ||
CDataObject *self = _CDataObject_CAST(myself); | ||
Py_ssize_t size; | ||
Py_ssize_t offset; | ||
PyObject *proto; | ||
void *deref = locked_deref(self); | ||
void *deref = *(void **)self->b_ptr; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Co-authored-by: Victor Stinner <vstinner@python.org>
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++) { |
There was a problem hiding this comment.
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++) {
There was a problem hiding this comment.
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.
This reverts commit 011c747.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Merged, thank you. I don't think that this change needs to be backported to 3.13. |
It can't; we haven't been backporting ctypes thread safety changes. |
We need to hold the lock as long as we plan on using the pointer, not just when we dereference it.
ctypes.Pointer.get_contents
doesn't hold the object's lock for long enough #131974