-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
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
Changes from 5 commits
0a27451
e2a4ea0
22004fb
8f52814
011c747
c31a405
7a44c77
9648678
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Fix several thread-safety issues in :mod:`ctypes` on the :term:`free | ||
| threaded <free threading>` build. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5326,13 +5326,13 @@ | |
| PyCPointer_Type | ||
| */ | ||
| static PyObject * | ||
| Pointer_item(PyObject *myself, Py_ssize_t index) | ||
| Pointer_item_lock_held(PyObject *myself, Py_ssize_t index) | ||
| { | ||
| 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; | ||
|
|
||
| if (deref == NULL) { | ||
| PyErr_SetString(PyExc_ValueError, | ||
|
|
@@ -5364,8 +5364,23 @@ | |
| index, size, (char *)((char *)deref + offset)); | ||
| } | ||
|
|
||
| static PyObject * | ||
| Pointer_item(PyObject *myself, Py_ssize_t index) | ||
| { | ||
| CDataObject *self = _CDataObject_CAST(myself); | ||
| PyObject *res; | ||
| // TODO: The plan is to make LOCK_PTR() a mutex instead of a critical | ||
| // section someday, so when that happens, this needs to get refactored | ||
kumaraditya303 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // to be re-entrant safe. | ||
| // This goes for all the locks here. | ||
| LOCK_PTR(self); | ||
| res = Pointer_item_lock_held(myself, index); | ||
| UNLOCK_PTR(self); | ||
| return res; | ||
| } | ||
|
|
||
| static int | ||
| Pointer_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value) | ||
| Pointer_ass_item_lock_held(PyObject *myself, Py_ssize_t index, PyObject *value) | ||
| { | ||
| CDataObject *self = _CDataObject_CAST(myself); | ||
| Py_ssize_t size; | ||
|
|
@@ -5378,7 +5393,7 @@ | |
| return -1; | ||
| } | ||
|
|
||
| void *deref = locked_deref(self); | ||
| void *deref = *(void **)self->b_ptr; | ||
| if (deref == NULL) { | ||
| PyErr_SetString(PyExc_ValueError, | ||
| "NULL pointer access"); | ||
|
|
@@ -5409,10 +5424,21 @@ | |
| index, size, ((char *)deref + offset)); | ||
| } | ||
|
|
||
| static int | ||
| Pointer_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value) | ||
| { | ||
| CDataObject *self = _CDataObject_CAST(myself); | ||
| int res; | ||
| LOCK_PTR(self); | ||
| res = Pointer_ass_item_lock_held(myself, index, value); | ||
| UNLOCK_PTR(self); | ||
| return res; | ||
| } | ||
|
|
||
| static PyObject * | ||
| Pointer_get_contents(PyObject *self, void *closure) | ||
| Pointer_get_contents_lock_held(PyObject *self, void *closure) | ||
| { | ||
| void *deref = locked_deref(_CDataObject_CAST(self)); | ||
| void *deref = *(void **)_CDataObject_CAST(self)->b_ptr; | ||
| if (deref == NULL) { | ||
| PyErr_SetString(PyExc_ValueError, | ||
| "NULL pointer access"); | ||
|
|
@@ -5429,6 +5455,17 @@ | |
| return PyCData_FromBaseObj(st, stginfo->proto, self, 0, deref); | ||
| } | ||
|
|
||
| static PyObject * | ||
| Pointer_get_contents(PyObject *myself, void *closure) | ||
| { | ||
| CDataObject *self = _CDataObject_CAST(myself); | ||
| PyObject *res; | ||
| LOCK_PTR(self); | ||
| res = Pointer_get_contents_lock_held(myself, closure); | ||
| UNLOCK_PTR(self); | ||
| return res; | ||
| } | ||
|
|
||
| static int | ||
| Pointer_set_contents(PyObject *op, PyObject *value, void *closure) | ||
| { | ||
|
|
@@ -5462,7 +5499,10 @@ | |
| } | ||
|
|
||
| dst = (CDataObject *)value; | ||
| assert(dst != self); // XXX Can the user do this? | ||
kumaraditya303 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| LOCK_PTR(dst); | ||
| locked_deref_assign(self, dst->b_ptr); | ||
| UNLOCK_PTR(dst); | ||
|
|
||
| /* | ||
| A Pointer instance must keep the value it points to alive. So, a | ||
|
|
@@ -5514,6 +5554,21 @@ | |
| return generic_pycdata_new(st, type, args, kw); | ||
| } | ||
|
|
||
| static int | ||
| copy_pointer_to_list_lock_held(PyObject *myself, PyObject *np, Py_ssize_t len, | ||
| Py_ssize_t start, Py_ssize_t step) | ||
| { | ||
| for (size_t cur = start, Py_ssize_t i = 0; i < len; cur += step, i++) { | ||
|
Check failure on line 5561 in Modules/_ctypes/_ctypes.c
|
||
|
||
| PyObject *v = Pointer_item_lock_held(myself, cur); | ||
|
Check failure on line 5562 in Modules/_ctypes/_ctypes.c
|
||
| if (!v) { | ||
| return -1; | ||
| } | ||
| PyList_SET_ITEM(np, i, v); | ||
|
Check failure on line 5566 in Modules/_ctypes/_ctypes.c
|
||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| static PyObject * | ||
| Pointer_subscript(PyObject *myself, PyObject *item) | ||
| { | ||
|
|
@@ -5595,14 +5650,14 @@ | |
| } | ||
| assert(iteminfo); | ||
| if (iteminfo->getfunc == _ctypes_get_fielddesc("c")->getfunc) { | ||
| char *ptr = locked_deref(self); | ||
| char *dest; | ||
|
|
||
| if (len <= 0) | ||
| return Py_GetConstant(Py_CONSTANT_EMPTY_BYTES); | ||
| if (step == 1) { | ||
| PyObject *res; | ||
| LOCK_PTR(self); | ||
| char *ptr = *(void **)self->b_ptr; | ||
| res = PyBytes_FromStringAndSize(ptr + start, | ||
| len); | ||
| UNLOCK_PTR(self); | ||
|
|
@@ -5612,6 +5667,7 @@ | |
| if (dest == NULL) | ||
| return PyErr_NoMemory(); | ||
| LOCK_PTR(self); | ||
| char *ptr = *(void **)self->b_ptr; | ||
| for (cur = start, i = 0; i < len; cur += step, i++) { | ||
| dest[i] = ptr[cur]; | ||
| } | ||
|
|
@@ -5621,14 +5677,14 @@ | |
| return np; | ||
| } | ||
| if (iteminfo->getfunc == _ctypes_get_fielddesc("u")->getfunc) { | ||
| wchar_t *ptr = locked_deref(self); | ||
| wchar_t *dest; | ||
|
|
||
| if (len <= 0) | ||
| return Py_GetConstant(Py_CONSTANT_EMPTY_STR); | ||
| if (step == 1) { | ||
| PyObject *res; | ||
| LOCK_PTR(self); | ||
| wchar_t *ptr = *(wchar_t **)self->b_ptr; | ||
| res = PyUnicode_FromWideChar(ptr + start, | ||
| len); | ||
| UNLOCK_PTR(self); | ||
|
|
@@ -5638,6 +5694,7 @@ | |
| if (dest == NULL) | ||
| return PyErr_NoMemory(); | ||
| LOCK_PTR(self); | ||
| wchar_t *ptr = *(wchar_t **)self->b_ptr; | ||
| for (cur = start, i = 0; i < len; cur += step, i++) { | ||
| dest[i] = ptr[cur]; | ||
| } | ||
|
|
@@ -5651,14 +5708,15 @@ | |
| if (np == NULL) | ||
| return NULL; | ||
|
|
||
| for (cur = start, i = 0; i < len; cur += step, i++) { | ||
| PyObject *v = Pointer_item(myself, cur); | ||
| if (!v) { | ||
| Py_DECREF(np); | ||
| return NULL; | ||
| } | ||
| PyList_SET_ITEM(np, i, v); | ||
| int res; | ||
| LOCK_PTR(self); | ||
| res = copy_pointer_to_list_lock_held(myself, np, len, start, step); | ||
| UNLOCK_PTR(self); | ||
| if (res < 0) { | ||
| Py_DECREF(np); | ||
| return NULL; | ||
| } | ||
|
|
||
| return np; | ||
| } | ||
| else { | ||
|
|
||
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.