Skip to content

gh-127945: fix critical sections around ctypes array #132646

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 17, 2025
Merged

gh-127945: fix critical sections around ctypes array #132646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Lib/test/test_ctypes/test_find.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import unittest.mock
from ctypes import CDLL, RTLD_GLOBAL
from ctypes.util import find_library
from test.support import os_helper
from test.support import os_helper, thread_unsafe


# On some systems, loading the OpenGL libraries needs the RTLD_GLOBAL mode.
Expand Down Expand Up @@ -78,6 +78,7 @@ def test_shell_injection(self):
@unittest.skipUnless(sys.platform.startswith('linux'),
'Test only valid for Linux')
class FindLibraryLinux(unittest.TestCase):
@thread_unsafe('uses setenv')
def test_find_on_libpath(self):
import subprocess
import tempfile
Expand Down
4 changes: 3 additions & 1 deletion Lib/test/test_ctypes/test_values.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from ctypes import (Structure, CDLL, POINTER, pythonapi,
_pointer_type_cache,
c_ubyte, c_char_p, c_int)
from test.support import import_helper
from test.support import import_helper, thread_unsafe


class ValuesTestCase(unittest.TestCase):
Expand All @@ -18,6 +18,7 @@ def setUp(self):
_ctypes_test = import_helper.import_module("_ctypes_test")
self.ctdll = CDLL(_ctypes_test.__file__)

@thread_unsafe("static global variables aren't thread-safe")
def test_an_integer(self):
# This test checks and changes an integer stored inside the
# _ctypes_test dll/shared lib.
Expand Down Expand Up @@ -46,6 +47,7 @@ def test_optimizeflag(self):
opt = c_int.in_dll(pythonapi, "Py_OptimizeFlag").value
self.assertEqual(opt, sys.flags.optimize)

@thread_unsafe('overrides frozen modules')
def test_frozentable(self):
# Python exports a PyImport_FrozenModules symbol. This is a
# pointer to an array of struct _frozen entries. The end of the
Expand Down
72 changes: 56 additions & 16 deletions Modules/_ctypes/_ctypes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4892,8 +4892,10 @@ Array_init(PyObject *self, PyObject *args, PyObject *kw)
}

static PyObject *
Array_item(PyObject *myself, Py_ssize_t index)
Array_item_lock_held(PyObject *myself, Py_ssize_t index)
{
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(myself);

CDataObject *self = _CDataObject_CAST(myself);
Py_ssize_t offset, size;

Expand All @@ -4920,8 +4922,20 @@ Array_item(PyObject *myself, Py_ssize_t index)
}

static PyObject *
Array_subscript(PyObject *myself, PyObject *item)
Array_item(PyObject *myself, Py_ssize_t index)
{
PyObject *result;
Py_BEGIN_CRITICAL_SECTION(myself);
result = Array_item_lock_held(myself, index);
Py_END_CRITICAL_SECTION();
return result;
}

static PyObject *
Array_subscript_lock_held(PyObject *myself, PyObject *item)
{
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(myself);

CDataObject *self = _CDataObject_CAST(myself);

if (PyIndex_Check(item)) {
Expand All @@ -4931,7 +4945,7 @@ Array_subscript(PyObject *myself, PyObject *item)
return NULL;
if (i < 0)
i += self->b_length;
return Array_item(myself, i);
return Array_item_lock_held(myself, i);
}
else if (PySlice_Check(item)) {
PyObject *proto;
Expand Down Expand Up @@ -4966,23 +4980,19 @@ Array_subscript(PyObject *myself, PyObject *item)
return Py_GetConstant(Py_CONSTANT_EMPTY_BYTES);
if (step == 1) {
PyObject *res;
Py_BEGIN_CRITICAL_SECTION(self);
res = PyBytes_FromStringAndSize(ptr + start,
slicelen);
Py_END_CRITICAL_SECTION();
return res;
}
dest = (char *)PyMem_Malloc(slicelen);

if (dest == NULL)
return PyErr_NoMemory();

Py_BEGIN_CRITICAL_SECTION(self);
for (cur = start, i = 0; i < slicelen;
cur += step, i++) {
dest[i] = ptr[cur];
}
Py_END_CRITICAL_SECTION();

np = PyBytes_FromStringAndSize(dest, slicelen);
PyMem_Free(dest);
Expand All @@ -4996,10 +5006,8 @@ Array_subscript(PyObject *myself, PyObject *item)
return Py_GetConstant(Py_CONSTANT_EMPTY_STR);
if (step == 1) {
PyObject *res;
Py_BEGIN_CRITICAL_SECTION(self);
res = PyUnicode_FromWideChar(ptr + start,
slicelen);
Py_END_CRITICAL_SECTION();
return res;
}

Expand All @@ -5009,12 +5017,10 @@ Array_subscript(PyObject *myself, PyObject *item)
return NULL;
}

Py_BEGIN_CRITICAL_SECTION(self);
for (cur = start, i = 0; i < slicelen;
cur += step, i++) {
dest[i] = ptr[cur];
}
Py_END_CRITICAL_SECTION();

np = PyUnicode_FromWideChar(dest, slicelen);
PyMem_Free(dest);
Expand All @@ -5027,7 +5033,7 @@ Array_subscript(PyObject *myself, PyObject *item)

for (cur = start, i = 0; i < slicelen;
cur += step, i++) {
PyObject *v = Array_item(myself, cur);
PyObject *v = Array_item_lock_held(myself, cur);
if (v == NULL) {
Py_DECREF(np);
return NULL;
Expand All @@ -5041,12 +5047,24 @@ Array_subscript(PyObject *myself, PyObject *item)
"indices must be integers");
return NULL;
}
}


static PyObject *
Array_subscript(PyObject *myself, PyObject *item)
{
PyObject *result;
Py_BEGIN_CRITICAL_SECTION(myself);
result = Array_subscript_lock_held(myself, item);
Py_END_CRITICAL_SECTION();
return result;
}

static int
Array_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value)
Array_ass_item_lock_held(PyObject *myself, Py_ssize_t index, PyObject *value)
{
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(myself);

CDataObject *self = _CDataObject_CAST(myself);
Py_ssize_t size, offset;
char *ptr;
Expand Down Expand Up @@ -5078,7 +5096,18 @@ Array_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value)
}

static int
Array_ass_subscript(PyObject *myself, PyObject *item, PyObject *value)
Array_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value)
{
int result;
Py_BEGIN_CRITICAL_SECTION(myself);
result = Array_ass_item_lock_held(myself, index, value);
Py_END_CRITICAL_SECTION();
return result;
}


static int
Array_ass_subscript_lock_held(PyObject *myself, PyObject *item, PyObject *value)
{
CDataObject *self = _CDataObject_CAST(myself);

Expand All @@ -5095,7 +5124,7 @@ Array_ass_subscript(PyObject *myself, PyObject *item, PyObject *value)
return -1;
if (i < 0)
i += self->b_length;
return Array_ass_item(myself, i, value);
return Array_ass_item_lock_held(myself, i, value);
}
else if (PySlice_Check(item)) {
Py_ssize_t start, stop, step, slicelen, otherlen, i;
Expand All @@ -5120,7 +5149,7 @@ Array_ass_subscript(PyObject *myself, PyObject *item, PyObject *value)
int result;
if (item == NULL)
return -1;
result = Array_ass_item(myself, cur, item);
result = Array_ass_item_lock_held(myself, cur, item);
Py_DECREF(item);
if (result == -1)
return -1;
Expand All @@ -5134,6 +5163,17 @@ Array_ass_subscript(PyObject *myself, PyObject *item, PyObject *value)
}
}

static int
Array_ass_subscript(PyObject *myself, PyObject *item, PyObject *value)
{
int result;
Py_BEGIN_CRITICAL_SECTION(myself);
result = Array_ass_subscript_lock_held(myself, item, value);
Py_END_CRITICAL_SECTION();
return result;
}


static Py_ssize_t
Array_length(PyObject *myself)
{
Expand Down
Loading