Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34.2k
gh-145272: Fix data race when accessing func_code in PyFunctionObject#145534
gh-145272: Fix data race when accessing func_code in PyFunctionObject#145534Krishna-web-hub wants to merge 3 commits intopython:mainfrom
Conversation
Uh oh!
There was an error while loading.Please reload this page.
vstinner left a comment
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.
Is this issue specific to func_code? What about otherPyFunction methods such as PyFunction_GET_GLOBALS() or PyFunction_GET_MODULE()?
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Krishna-web-hub commentedMar 6, 2026
@vstinner actually this issue was mentioned only to fixation of the func_code. But the func_code have high chance of being reassigned at runtime. But others like func_globals and func_module are set at the time of the function creation. But there are few objects like func_annotations, func_defaults they can cause the race situation but i think applying the atomic pointer fix can slow down the cpython. Based on this i want to know your opinion on what i should be doing next ? |
| PyCodeObject *code = _Py_atomic_load_ptr(&op->func_code); | ||
| return Py_NewRef(code); |
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.
This isn't right.
code might have been deallocated by the time you doPy_NewRef
| @@ -0,0 +1 @@ | |||
| Fixing race condition in PyFunctionObject.func_code | |||
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.
I think we should be less precise in terms of implementation and rather mention__func__.__code__ directly.
picnixz left a comment
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.
But others like func_globals and func_module are set at the time of the function creation. But there are few objects like func_annotations, func_defaults they can cause the race situation but i think applying the atomic pointer fix can slow down the cpython
Regular build shouldn't be affected if add a Py_GIL_DISABLED guard. But I would indeed have a macro that atomically access attributes instead.
| } | ||
| PyObject *func_code = PyFunction_GET_CODE(op); | ||
| PyCodeObject *func_code =(PyCodeObject *)PyFunction_GET_CODE(op); |
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.
Why this change?
| return_PyFunction_CAST(func)->func_code; | ||
| PyFunctionObject*op=_PyFunction_CAST(func); | ||
| #ifdefPy_GIL_DISABLED | ||
| return (PyObject*)_Py_atomic_load_ptr(&op->func_code); |
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.
Aren'tall attributes of function objects subject to this issue then?
| handle_func_event(PyFunction_EVENT_MODIFY_CODE, op, value); | ||
| _PyFunction_ClearVersion(op); | ||
| Py_XSETREF(op->func_code, Py_NewRef(value)); | ||
| PyCodeObject *new = (PyCodeObject *)Py_NewRef(value); |
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.
This should be GIL_DISABLED-guarded
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.
Also, don't we have a FT-helper alterantive to Py_XSETREF?
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Uh oh!
There was an error while loading.Please reload this page.
Fix a data race when accessing PyFunctionObject.func_code in free-threaded builds.
This patch replaces the direct read with
_Py_atomic_load_ptr()and updates the setter to use_Py_atomic_exchange_ptr()so that updates tofunc_codeare performed atomically.The race was reproduced using
ThreadSanitizerand verified to be fixed after this change.func_set_code#145272