Skip to content

Commit 666bb8e

Browse files
colesburycolesbury
committed
pythongh-117323: Make cell thread-safe in free-threaded builds
1 parent 8dbfdb2 commit 666bb8e

File tree

10 files changed

+82
-41
lines changed

10 files changed

+82
-41
lines changed

‎Include/internal/pycore_cell.h

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
#ifndef Py_INTERNAL_CELL_H
2+
#define Py_INTERNAL_CELL_H
3+
4+
#include "pycore_critical_section.h"
5+
6+
#ifdef __cplusplus
7+
extern "C" {
8+
#endif
9+
10+
#ifndef Py_BUILD_CORE
11+
# error "this header requires Py_BUILD_CORE define"
12+
#endif
13+
14+
// Sets the cell contents to `value` and return previous contents. Steals a
15+
// reference to `value`.
16+
static inline PyObject *
17+
PyCell_SwapTakeRef(PyCellObject *cell, PyObject *value)
18+
{
19+
PyObject *old_value;
20+
Py_BEGIN_CRITICAL_SECTION(cell);
21+
old_value = cell->ob_ref;
22+
cell->ob_ref = value;
23+
Py_END_CRITICAL_SECTION();
24+
return old_value;
25+
}
26+
27+
static inline void
28+
PyCell_SetTakeRef(PyCellObject *cell, PyObject *value)
29+
{
30+
PyObject *old_value = PyCell_SwapTakeRef(cell, value);
31+
Py_XDECREF(old_value);
32+
}
33+
34+
// Gets the cell contents. Returns a new reference.
35+
static inline PyObject *
36+
PyCell_GetRef(PyCellObject *cell)
37+
{
38+
PyObject *res;
39+
Py_BEGIN_CRITICAL_SECTION(cell);
40+
res = Py_XNewRef(cell->ob_ref);
41+
Py_END_CRITICAL_SECTION();
42+
return res;
43+
}
44+
45+
#ifdef __cplusplus
46+
}
47+
#endif
48+
#endif /* !Py_INTERNAL_CELL_H */

‎Makefile.pre.in

+1
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,7 @@ PYTHON_HEADERS= \
11301130
$(srcdir)/Include/internal/pycore_bytesobject.h \
11311131
$(srcdir)/Include/internal/pycore_call.h \
11321132
$(srcdir)/Include/internal/pycore_capsule.h \
1133+
$(srcdir)/Include/internal/pycore_cell.h \
11331134
$(srcdir)/Include/internal/pycore_ceval.h \
11341135
$(srcdir)/Include/internal/pycore_ceval_state.h \
11351136
$(srcdir)/Include/internal/pycore_code.h \

‎Objects/cellobject.c

+3-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* Cell object implementation */
22

33
#include "Python.h"
4+
#include "pycore_cell.h" // PyCell_GetRef()
45
#include "pycore_modsupport.h" // _PyArg_NoKeywords()
56
#include "pycore_object.h"
67

@@ -56,8 +57,7 @@ PyCell_Get(PyObject *op)
5657
PyErr_BadInternalCall();
5758
return NULL;
5859
}
59-
PyObject *value = PyCell_GET(op);
60-
return Py_XNewRef(value);
60+
return PyCell_GetRef((PyCellObject *)op);
6161
}
6262

6363
int
@@ -67,9 +67,7 @@ PyCell_Set(PyObject *op, PyObject *value)
6767
PyErr_BadInternalCall();
6868
return -1;
6969
}
70-
PyObject *old_value = PyCell_GET(op);
71-
PyCell_SET(op, Py_XNewRef(value));
72-
Py_XDECREF(old_value);
70+
PyCell_SetTakeRef((PyCellObject *)op, Py_XNewRef(value));
7371
return 0;
7472
}
7573

‎PCbuild/pythoncore.vcxproj

+1
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@
210210
<ClInclude Include="..\Include\internal\pycore_bytesobject.h" />
211211
<ClInclude Include="..\Include\internal\pycore_call.h" />
212212
<ClInclude Include="..\Include\internal\pycore_capsule.h" />
213+
<ClInclude Include="..\Include\internal\pycore_cell.h" />
213214
<ClInclude Include="..\Include\internal\pycore_ceval.h" />
214215
<ClInclude Include="..\Include\internal\pycore_ceval_state.h" />
215216
<ClInclude Include="..\Include\internal\pycore_cfg.h" />

‎PCbuild/pythoncore.vcxproj.filters

+3
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,9 @@
555555
<ClInclude Include="..\Include\internal\pycore_capsule.h">
556556
<Filter>Include\internal</Filter>
557557
</ClInclude>
558+
<ClInclude Include="..\Include\internal\pycore_cell.h">
559+
<Filter>Include\internal</Filter>
560+
</ClInclude>
558561
<ClInclude Include="..\Include\internal\pycore_ceval.h">
559562
<Filter>Include\internal</Filter>
560563
</ClInclude>

‎Python/bytecodes.c

+8-12
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "Python.h"
1010
#include "pycore_abstract.h" // _PyIndex_Check()
11+
#include "pycore_cell.h" // PyCell_GetRef()
1112
#include "pycore_code.h"
1213
#include "pycore_emscripten_signal.h" // _Py_CHECK_EMSCRIPTEN_SIGNALS
1314
#include "pycore_function.h"
@@ -1523,14 +1524,13 @@ dummy_func(
15231524

15241525
inst(DELETE_DEREF, (--)) {
15251526
PyObject *cell = GETLOCAL(oparg);
1526-
PyObject *oldobj = PyCell_GET(cell);
15271527
// Can't use ERROR_IF here.
15281528
// Fortunately we don't need its superpower.
1529+
PyObject *oldobj = PyCell_SwapTakeRef((PyCellObject *)cell, NULL);
15291530
if (oldobj == NULL) {
15301531
_PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg);
15311532
ERROR_NO_POP();
15321533
}
1533-
PyCell_SET(cell, NULL);
15341534
Py_DECREF(oldobj);
15351535
}
15361536

@@ -1543,32 +1543,28 @@ dummy_func(
15431543
ERROR_NO_POP();
15441544
}
15451545
if (!value) {
1546-
PyObject *cell = GETLOCAL(oparg);
1547-
value = PyCell_GET(cell);
1546+
PyCellObject *cell = (PyCellObject *)GETLOCAL(oparg);
1547+
value = PyCell_GetRef(cell);
15481548
if (value == NULL) {
15491549
_PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg);
15501550
ERROR_NO_POP();
15511551
}
1552-
Py_INCREF(value);
15531552
}
15541553
Py_DECREF(class_dict);
15551554
}
15561555

15571556
inst(LOAD_DEREF, ( -- value)) {
1558-
PyObject *cell = GETLOCAL(oparg);
1559-
value = PyCell_GET(cell);
1557+
PyCellObject *cell = (PyCellObject *)GETLOCAL(oparg);
1558+
value = PyCell_GetRef(cell);
15601559
if (value == NULL) {
15611560
_PyEval_FormatExcUnbound(tstate, _PyFrame_GetCode(frame), oparg);
15621561
ERROR_IF(true, error);
15631562
}
1564-
Py_INCREF(value);
15651563
}
15661564

15671565
inst(STORE_DEREF, (v --)) {
1568-
PyObject *cell = GETLOCAL(oparg);
1569-
PyObject *oldobj = PyCell_GET(cell);
1570-
PyCell_SET(cell, v);
1571-
Py_XDECREF(oldobj);
1566+
PyCellObject *cell = (PyCellObject *)GETLOCAL(oparg);
1567+
PyCell_SetTakeRef(cell, v);
15721568
}
15731569

15741570
inst(COPY_FREE_VARS, (--)) {

‎Python/ceval.c

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "Python.h"
66
#include "pycore_abstract.h" // _PyIndex_Check()
77
#include "pycore_call.h" // _PyObject_CallNoArgs()
8+
#include "pycore_cell.h" // PyCell_GetRef()
89
#include "pycore_ceval.h"
910
#include "pycore_code.h"
1011
#include "pycore_emscripten_signal.h" // _Py_CHECK_EMSCRIPTEN_SIGNALS

‎Python/executor_cases.c.h

+7-12
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/generated_cases.c.h

+7-12
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Tools/cases_generator/analyzer.py

+3
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,9 @@ def compute_properties(op: parser.InstDef) -> Properties:
522522
variable_used(op, "PyCell_New")
523523
or variable_used(op, "PyCell_GET")
524524
or variable_used(op, "PyCell_SET")
525+
or variable_used(op, "PyCell_GetRef")
526+
or variable_used(op, "PyCell_SetTakeRef")
527+
or variable_used(op, "PyCell_SwapTakeRef")
525528
)
526529
deopts_if = variable_used(op, "DEOPT_IF")
527530
exits_if = variable_used(op, "EXIT_IF")

0 commit comments

Comments
 (0)