Skip to content

Commit 3f4f99f

Browse files
yoneymeta-codesync[bot]
yoney
authored and
meta-codesync[bot]
committed
Make frame evaluator identity thread-safe
Summary: Concurrent JIT compilation can call Ci_InitFrameEvalFunc() from multiple threads, racing on Ci_EvalFrameFunc. Reviewed By: DinoV Differential Revision: D115476023 fbshipit-source-id: 9b3680b01ef2c3390e269ab516e38f19cbe6c81f
1 parent 18cd69d commit 3f4f99f

13 files changed

Lines changed: 44 additions & 69 deletions

‎cinderx/Interpreter/interpreter_base.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,8 @@ int Ci_InitFrameEvalFunc() {
1919
#ifdef ENABLE_EVAL_HOOK
2020
Ci_hook_EvalFrame = Ci_EvalFrame;
2121
#elif defined(ENABLE_PEP523_HOOK)
22-
// Let borrowed.h know the eval frame pointer
23-
Ci_EvalFrameFunc = Ci_EvalFrame;
24-
22+
// Allow borrowed specialization code to recognize CinderX's evaluator.
23+
Ci_SetEvalFrameFunc(Ci_EvalFrame);
2524
auto interp = _PyInterpreterState_GET();
2625
auto current_eval_frame = _PyInterpreterState_GetEvalFrameFunc(interp);
2726
if (current_eval_frame == Ci_EvalFrame) {
@@ -52,6 +51,7 @@ void Ci_FiniFrameEvalFunc() {
5251
Ci_hook_EvalFrame = nullptr;
5352
#elif defined(ENABLE_PEP523_HOOK)
5453
_PyInterpreterState_SetEvalFrameFunc(_PyInterpreterState_GET(), nullptr);
54+
Ci_SetEvalFrameFunc(nullptr);
5555
#endif
5656
#endif
5757
}

‎cinderx/UpstreamBorrow/UpstreamBorrow.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ def transform_eval_frame(lines: list[str]) -> list[str]:
181181
return [
182182
line.replace(
183183
"_PyInterpreterState_GET()->eval_frame",
184-
"(_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)",
184+
"(_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())",
185185
)
186186
for line in lines
187187
]

‎cinderx/UpstreamBorrow/borrowed-3.14.c.template‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,6 @@ _PyType_LookupRefAndVersion(PyTypeObject *type, PyObject *name, unsigned int *ve
7171
return res;
7272
}
7373

74-
#ifdef ENABLE_PEP523_HOOK
75-
_PyFrameEvalFunction Ci_EvalFrameFunc;
76-
#else
77-
#define Ci_EvalFrameFunc NULL
78-
#endif
79-
8074
// In 3.12 _PyAsyncGenValueWrapperNew needs thread-state. As this is used from
8175
// the JIT we could get the value from the thread-state register. This would be
8276
// slightly more efficient, but quite a bit more work and async-generators are

‎cinderx/UpstreamBorrow/borrowed-3.14.free-threading.c.template‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,6 @@ ASSERT_DICT_LOCKED(PyObject *op)
8282
}
8383
#endif
8484

85-
#ifdef ENABLE_PEP523_HOOK
86-
_PyFrameEvalFunction Ci_EvalFrameFunc;
87-
#else
88-
#define Ci_EvalFrameFunc NULL
89-
#endif
90-
9185
// In 3.12 _PyAsyncGenValueWrapperNew needs thread-state. As this is used from
9286
// the JIT we could get the value from the thread-state register. This would be
9387
// slightly more efficient, but quite a bit more work and async-generators are

‎cinderx/UpstreamBorrow/borrowed-3.14.free-threading.gen_cached.c‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,6 @@ ASSERT_DICT_LOCKED(PyObject *op)
8585
}
8686
#endif
8787

88-
#ifdef ENABLE_PEP523_HOOK
89-
_PyFrameEvalFunction Ci_EvalFrameFunc;
90-
#else
91-
#define Ci_EvalFrameFunc NULL
92-
#endif
93-
9488
// In 3.12 _PyAsyncGenValueWrapperNew needs thread-state. As this is used from
9589
// the JIT we could get the value from the thread-state register. This would be
9690
// slightly more efficient, but quite a bit more work and async-generators are
@@ -5577,7 +5571,7 @@ do_specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject*
55775571
return -1;
55785572
}
55795573
/* Don't specialize if PEP 523 is active */
5580-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
5574+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
55815575
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
55825576
return -1;
55835577
}
@@ -5652,7 +5646,7 @@ do_specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject*
56525646
return -1;
56535647
}
56545648
/* Don't specialize if PEP 523 is active */
5655-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
5649+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
56565650
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
56575651
return -1;
56585652
}
@@ -6054,7 +6048,7 @@ specialize_py_call(PyFunctionObject *func, _Py_CODEUNIT *instr, int nargs,
60546048
PyCodeObject *code = (PyCodeObject *)func->func_code;
60556049
int kind = function_kind(code);
60566050
/* Don't specialize if PEP 523 is active */
6057-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
6051+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
60586052
SPECIALIZATION_FAIL(CALL, SPEC_FAIL_CALL_PEP_523);
60596053
return -1;
60606054
}
@@ -6258,7 +6252,7 @@ specialize_py_call_kw(PyFunctionObject *func, _Py_CODEUNIT *instr, int nargs,
62586252
PyCodeObject *code = (PyCodeObject *)func->func_code;
62596253
int kind = function_kind(code);
62606254
/* Don't specialize if PEP 523 is active */
6261-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
6255+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
62626256
SPECIALIZATION_FAIL(CALL, SPEC_FAIL_CALL_PEP_523);
62636257
return -1;
62646258
}
@@ -6536,7 +6530,7 @@ _Py_Specialize_BinaryOp(_PyStackRef lhs_st, _PyStackRef rhs_st, _Py_CODEUNIT *in
65366530
PyHeapTypeObject *ht = (PyHeapTypeObject *)container_type;
65376531
if (kind == SIMPLE_FUNCTION &&
65386532
fcode->co_argcount == 2 &&
6539-
!(_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc) && /* Don't specialize if PEP 523 is active */
6533+
!(_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc()) && /* Don't specialize if PEP 523 is active */
65406534
_PyType_CacheGetItemForSpecialization(ht, descriptor, (uint32_t)tp_version))
65416535
{
65426536
specialize(instr, BINARY_OP_SUBSCR_GETITEM);
@@ -6684,7 +6678,7 @@ _Py_Specialize_ForIter(_PyStackRef iter, _Py_CODEUNIT *instr, int oparg)
66846678
instr[oparg + INLINE_CACHE_ENTRIES_FOR_ITER + 1].op.code == INSTRUMENTED_END_FOR
66856679
);
66866680
/* Don't specialize if PEP 523 is active */
6687-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc))
6681+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc()))
66886682
goto failure;
66896683
specialize(instr, FOR_ITER_GEN);
66906684
return;
@@ -6704,7 +6698,7 @@ _Py_Specialize_Send(_PyStackRef receiver_st, _Py_CODEUNIT *instr)
67046698
PyTypeObject *tp = Py_TYPE(receiver);
67056699
if (tp == &PyGen_Type || tp == &PyCoro_Type) {
67066700
/* Don't specialize if PEP 523 is active */
6707-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
6701+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
67086702
SPECIALIZATION_FAIL(SEND, SPEC_FAIL_OTHER);
67096703
goto failure;
67106704
}

‎cinderx/UpstreamBorrow/borrowed-3.14.gen_cached.c‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,6 @@ _PyType_LookupRefAndVersion(PyTypeObject *type, PyObject *name, unsigned int *ve
7474
return res;
7575
}
7676

77-
#ifdef ENABLE_PEP523_HOOK
78-
_PyFrameEvalFunction Ci_EvalFrameFunc;
79-
#else
80-
#define Ci_EvalFrameFunc NULL
81-
#endif
82-
8377
// In 3.12 _PyAsyncGenValueWrapperNew needs thread-state. As this is used from
8478
// the JIT we could get the value from the thread-state register. This would be
8579
// slightly more efficient, but quite a bit more work and async-generators are
@@ -5693,7 +5687,7 @@ do_specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject*
56935687
return -1;
56945688
}
56955689
/* Don't specialize if PEP 523 is active */
5696-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
5690+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
56975691
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
56985692
return -1;
56995693
}
@@ -5768,7 +5762,7 @@ do_specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject*
57685762
return -1;
57695763
}
57705764
/* Don't specialize if PEP 523 is active */
5771-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
5765+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
57725766
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
57735767
return -1;
57745768
}
@@ -6170,7 +6164,7 @@ specialize_py_call(PyFunctionObject *func, _Py_CODEUNIT *instr, int nargs,
61706164
PyCodeObject *code = (PyCodeObject *)func->func_code;
61716165
int kind = function_kind(code);
61726166
/* Don't specialize if PEP 523 is active */
6173-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
6167+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
61746168
SPECIALIZATION_FAIL(CALL, SPEC_FAIL_CALL_PEP_523);
61756169
return -1;
61766170
}
@@ -6374,7 +6368,7 @@ specialize_py_call_kw(PyFunctionObject *func, _Py_CODEUNIT *instr, int nargs,
63746368
PyCodeObject *code = (PyCodeObject *)func->func_code;
63756369
int kind = function_kind(code);
63766370
/* Don't specialize if PEP 523 is active */
6377-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
6371+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
63786372
SPECIALIZATION_FAIL(CALL, SPEC_FAIL_CALL_PEP_523);
63796373
return -1;
63806374
}
@@ -6652,7 +6646,7 @@ _Py_Specialize_BinaryOp(_PyStackRef lhs_st, _PyStackRef rhs_st, _Py_CODEUNIT *in
66526646
PyHeapTypeObject *ht = (PyHeapTypeObject *)container_type;
66536647
if (kind == SIMPLE_FUNCTION &&
66546648
fcode->co_argcount == 2 &&
6655-
!(_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc) && /* Don't specialize if PEP 523 is active */
6649+
!(_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc()) && /* Don't specialize if PEP 523 is active */
66566650
_PyType_CacheGetItemForSpecialization(ht, descriptor, (uint32_t)tp_version))
66576651
{
66586652
specialize(instr, BINARY_OP_SUBSCR_GETITEM);
@@ -6800,7 +6794,7 @@ _Py_Specialize_ForIter(_PyStackRef iter, _Py_CODEUNIT *instr, int oparg)
68006794
instr[oparg + INLINE_CACHE_ENTRIES_FOR_ITER + 1].op.code == INSTRUMENTED_END_FOR
68016795
);
68026796
/* Don't specialize if PEP 523 is active */
6803-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc))
6797+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc()))
68046798
goto failure;
68056799
specialize(instr, FOR_ITER_GEN);
68066800
return;
@@ -6820,7 +6814,7 @@ _Py_Specialize_Send(_PyStackRef receiver_st, _Py_CODEUNIT *instr)
68206814
PyTypeObject *tp = Py_TYPE(receiver);
68216815
if (tp == &PyGen_Type || tp == &PyCoro_Type) {
68226816
/* Don't specialize if PEP 523 is active */
6823-
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_EvalFrameFunc)) {
6817+
if ((_PyInterpreterState_GET()->eval_frame != NULL && _PyInterpreterState_GET()->eval_frame != Ci_GetEvalFrameFunc())) {
68246818
SPECIALIZATION_FAIL(SEND, SPEC_FAIL_OTHER);
68256819
goto failure;
68266820
}

‎cinderx/UpstreamBorrow/borrowed-3.15.c.template‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,6 @@
1616

1717
getattrofunc Ci_tp_getattr_hook, Ci_tp_getattro;
1818

19-
#ifdef ENABLE_PEP523_HOOK
20-
_PyFrameEvalFunction Ci_EvalFrameFunc;
21-
#else
22-
#define Ci_EvalFrameFunc NULL
23-
#endif
24-
2519
PyObject* Cix_PyAsyncGenValueWrapperNew(PyObject* value) {
2620
return _PyAsyncGenValueWrapperNew(PyThreadState_GET(), value);
2721
}

‎cinderx/UpstreamBorrow/borrowed-3.15.gen_cached.c‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,6 @@
1919

2020
getattrofunc Ci_tp_getattr_hook, Ci_tp_getattro;
2121

22-
#ifdef ENABLE_PEP523_HOOK
23-
_PyFrameEvalFunction Ci_EvalFrameFunc;
24-
#else
25-
#define Ci_EvalFrameFunc NULL
26-
#endif
27-
2822
PyObject* Cix_PyAsyncGenValueWrapperNew(PyObject* value) {
2923
return _PyAsyncGenValueWrapperNew(PyThreadState_GET(), value);
3024
}

‎cinderx/UpstreamBorrow/borrowed-3.16.c.template‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,6 @@
1616

1717
getattrofunc Ci_tp_getattr_hook, Ci_tp_getattro;
1818

19-
#ifdef ENABLE_PEP523_HOOK
20-
_PyFrameEvalFunction Ci_EvalFrameFunc;
21-
#else
22-
#define Ci_EvalFrameFunc NULL
23-
#endif
24-
2519
PyObject* Cix_PyAsyncGenValueWrapperNew(PyObject* value) {
2620
return _PyAsyncGenValueWrapperNew(PyThreadState_GET(), value);
2721
}

‎cinderx/UpstreamBorrow/borrowed-3.16.gen_cached.c‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,6 @@
1919

2020
getattrofunc Ci_tp_getattr_hook, Ci_tp_getattro;
2121

22-
#ifdef ENABLE_PEP523_HOOK
23-
_PyFrameEvalFunction Ci_EvalFrameFunc;
24-
#else
25-
#define Ci_EvalFrameFunc NULL
26-
#endif
27-
2822
PyObject* Cix_PyAsyncGenValueWrapperNew(PyObject* value) {
2923
return _PyAsyncGenValueWrapperNew(PyThreadState_GET(), value);
3024
}

0 commit comments

Comments
 (0)