gh-141563: make PyDateTime_IMPORT thread-safe
#144210
Conversation
kumaraditya303
force-pushed
the
datetime
branch
2 times, most recently
from
ea5a3ea to
a6f6595
Compare
kumaraditya303
force-pushed
the
datetime
branch
2 times, most recently
from
3b08a06 to
96a62cb
Compare
kumaraditya303
force-pushed
the
datetime
branch
from
96a62cb to
36a06c8
Compare
|
|
||
| #define PyDateTime_IMPORT \ | ||
| PyDateTimeAPI = (PyDateTime_CAPI *)PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0) | ||
| static inline PyDateTime_CAPI * |
There was a problem hiding this comment.
Is it important that the function is a static inline function? Declaring the function as an "opaque" function with PyAPI_FUNC() would hide implementation details (move it to Modules/_datetimemodule.c).
There was a problem hiding this comment.
Is it important that the function is a static inline function?
No, it was just easier to do it this way, OTOH PyDateTimeAPI is already exposed so moving this to a api function doesn't really hides any implementation details.
There was a problem hiding this comment.
I'm a little bit worried by the usage of _Py_atomic in the public C API. I'm not sure that it's available in all compilers/use cases. Currently, it's only used if Py_GIL_DISABLED is defined by a few header files (ex: dictobject.h, refcount.h).
There was a problem hiding this comment.
'm not sure that it's available in all compilers/use cases. Currently, it's only used if Py_GIL_DISABLED is defined by a few header files (ex: dictobject.h, refcount.h).
No, see pylock.h. It is used in normal builds as well for PyMutex_Lock
There was a problem hiding this comment.
It seems fine to me to move it to a .c file. The function will need to take a pointer &PyDateTimeAPI though because PyDateTimeAPI is a static variable declared in a header file (ugh), so there's a unique variable per translation unit.
There was a problem hiding this comment.
No, see pylock.h. It is used in normal builds as well for PyMutex_Lock
Oh, I didn't notice these functions, you're right. In this case, I have no longer concerns about _Py_atomic usage in a static inline function.
3 participants
datetimeC API is not thread-safe #141563