Skip to content

Commit e40c36c

Browse files
authored
[mypyc] Fix list.pop primitive on free-threaded builds (#19522)
The old implementation caused segfaults on free-threaded builds. Provide a slower but working implementation for free-threaded builds. Also improve test coverage, since it used to be spotty. Tested on a manually compiled Python 3.14.0b4 free-threaded build on macOS.
1 parent 3a2b788 commit e40c36c

File tree

3 files changed

+58
-9
lines changed

3 files changed

+58
-9
lines changed

mypyc/lib-rt/list_ops.c

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,19 +235,51 @@ void CPyList_SetItemUnsafe(PyObject *list, Py_ssize_t index, PyObject *value) {
235235
PyList_SET_ITEM(list, index, value);
236236
}
237237

238-
PyObject *CPyList_PopLast(PyObject *obj)
238+
#ifdef Py_GIL_DISABLED
239+
// The original optimized list.pop implementation doesn't work on free-threaded
240+
// builds, so provide an alternative that is a bit slower but works.
241+
//
242+
// Note that this implementation isn't intended to be atomic.
243+
static inline PyObject *list_pop_index(PyObject *list, Py_ssize_t index) {
244+
PyObject *item = PyList_GetItemRef(list, index);
245+
if (item == NULL) {
246+
return NULL;
247+
}
248+
if (PySequence_DelItem(list, index) < 0) {
249+
Py_DECREF(item);
250+
return NULL;
251+
}
252+
return item;
253+
}
254+
#endif
255+
256+
PyObject *CPyList_PopLast(PyObject *list)
239257
{
258+
#ifdef Py_GIL_DISABLED
259+
// The other implementation causes segfaults on a free-threaded Python 3.14b4 build.
260+
Py_ssize_t index = PyList_GET_SIZE(list) - 1;
261+
return list_pop_index(list, index);
262+
#else
240263
// I tried a specalized version of pop_impl for just removing the
241264
// last element and it wasn't any faster in microbenchmarks than
242265
// the generic one so I ditched it.
243-
return list_pop_impl((PyListObject *)obj, -1);
266+
return list_pop_impl((PyListObject *)list, -1);
267+
#endif
244268
}
245269

246270
PyObject *CPyList_Pop(PyObject *obj, CPyTagged index)
247271
{
248272
if (CPyTagged_CheckShort(index)) {
249273
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
274+
#ifdef Py_GIL_DISABLED
275+
// We must use a slower implementation on free-threaded builds.
276+
if (n < 0) {
277+
n += PyList_GET_SIZE(obj);
278+
}
279+
return list_pop_index(obj, n);
280+
#else
250281
return list_pop_impl((PyListObject *)obj, n);
282+
#endif
251283
} else {
252284
PyErr_SetString(PyExc_OverflowError, CPYTHON_LARGE_INT_ERRMSG);
253285
return NULL;

mypyc/primitives/list_ops.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@
219219
)
220220

221221
# list.pop(index)
222-
list_pop = method_op(
222+
method_op(
223223
name="pop",
224224
arg_types=[list_rprimitive, int_rprimitive],
225225
return_type=object_rprimitive,

mypyc/test-data/run-lists.test

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,9 @@ print(primes(13))
150150
\[0, 0, 1, 1]
151151
\[0, 0, 1, 1, 0, 1, 0, 1, 0, 0, 0, 1, 0, 1]
152152

153-
[case testListBuild]
153+
[case testListPrimitives]
154+
from testutil import assertRaises
155+
154156
def test_list_build() -> None:
155157
# Currently LIST_BUILDING_EXPANSION_THRESHOLD equals to 10
156158
# long list built by list_build_op
@@ -169,9 +171,6 @@ def test_list_build() -> None:
169171
l3.append('a')
170172
assert l3 == ['a']
171173

172-
[case testListPrims]
173-
from typing import List
174-
175174
def test_append() -> None:
176175
l = [1, 2]
177176
l.append(10)
@@ -189,10 +188,28 @@ def test_pop_last() -> None:
189188

190189
def test_pop_index() -> None:
191190
l = [1, 2, 10, 3]
192-
l.pop(2)
191+
assert l.pop(2) == 10
193192
assert l == [1, 2, 3]
194-
l.pop(-2)
193+
assert l.pop(-2) == 2
195194
assert l == [1, 3]
195+
assert l.pop(-2) == 1
196+
assert l.pop(0) == 3
197+
assert l == []
198+
l = [int() + 1000, int() + 1001, int() + 1002]
199+
assert l.pop(0) == 1000
200+
assert l.pop(-1) == 1002
201+
assert l == [1001]
202+
203+
def test_pop_index_errors() -> None:
204+
l = [int() + 1000]
205+
with assertRaises(IndexError):
206+
l.pop(1)
207+
with assertRaises(IndexError):
208+
l.pop(-2)
209+
with assertRaises(OverflowError):
210+
l.pop(1 << 100)
211+
with assertRaises(OverflowError):
212+
l.pop(-(1 << 100))
196213

197214
def test_count() -> None:
198215
l = [1, 3]

0 commit comments

Comments
 (0)