From 4c1325e3b4afc348f08bc69cae3b0339118c5e8e Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sun, 28 Apr 2024 07:06:35 +0900 Subject: [PATCH 01/18] set capi-struct pointer to interp state --- Include/datetime.h | 25 +++++++++++++++++------ Include/internal/pycore_interp.h | 1 + Lib/test/test_capi/test_misc.py | 16 +++++++++++++++ Modules/_datetimemodule.c | 15 ++++++++++++++ Modules/_testmultiphase.c | 35 ++++++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 6 deletions(-) diff --git a/Include/datetime.h b/Include/datetime.h index b78cc0e8e2e5ac..abb2295b17099c 100644 --- a/Include/datetime.h +++ b/Include/datetime.h @@ -155,7 +155,7 @@ typedef struct /* Define structure for C API. */ -typedef struct { +typedef struct _pydatetime_capi { /* type objects */ PyTypeObject *DateType; PyTypeObject *DateTimeType; @@ -182,7 +182,8 @@ typedef struct { PyObject *(*DateTime_FromDateAndTimeAndFold)(int, int, int, int, int, int, int, PyObject*, int, PyTypeObject*); PyObject *(*Time_FromTimeAndFold)(int, int, int, int, PyObject*, int, PyTypeObject*); - + struct _pydatetime_capi *(*_get_capi_by_interp)(void); + void (*_set_capi_by_interp)(struct _pydatetime_capi *); } PyDateTime_CAPI; #define PyDateTime_CAPSULE_NAME "datetime.datetime_CAPI" @@ -194,10 +195,22 @@ typedef struct { * */ #ifndef _PY_DATETIME_IMPL /* Define global variable for the C API and a macro for setting it. */ -static PyDateTime_CAPI *PyDateTimeAPI = NULL; - -#define PyDateTime_IMPORT \ - PyDateTimeAPI = (PyDateTime_CAPI *)PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0) +static PyDateTime_CAPI * +_get_pydatetime_capi_dummy(void) +{ + return NULL; +} +static PyDateTime_CAPI *(*_get_pydatetime_capi)(void) = _get_pydatetime_capi_dummy; +#define PyDateTimeAPI _get_pydatetime_capi() + +static inline void pydatetime_import(void) { + PyDateTime_CAPI *capi = PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0); + if (capi) { + capi->_set_capi_by_interp(capi); + _get_pydatetime_capi = capi->_get_capi_by_interp; + } +} +#define PyDateTime_IMPORT pydatetime_import() /* Macro for access to the UTC singleton */ #define PyDateTime_TimeZone_UTC PyDateTimeAPI->TimeZone_UTC diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index d38959e3ce4ec5..eb21e10fb224ee 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -236,6 +236,7 @@ struct _is { // more comments. struct _obmalloc_state *obmalloc; + void *datetime_capi; PyObject *audit_hooks; PyType_WatchCallback type_watchers[TYPE_MAX_WATCHERS]; PyCode_WatchCallback code_watchers[CODE_MAX_WATCHERS]; diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 020e8493e57c0c..98849b8d179b83 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2282,6 +2282,22 @@ def test_module_state_shared_in_global(self): subinterp_attr_id = os.read(r, 100) self.assertEqual(main_attr_id, subinterp_attr_id) + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_datetime_capi_client(self): + script = textwrap.dedent(""" + import importlib.machinery + import importlib.util + fullname = '_test_datetime_capi_client' + origin = importlib.util.find_spec('_testmultiphase').origin + loader = importlib.machinery.ExtensionFileLoader(fullname, origin) + spec = importlib.util.spec_from_loader(fullname, loader) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + """) + exec(script) + ret = support.run_in_subinterp(script) + self.assertEqual(ret, 0) + @requires_subinterpreters class InterpreterConfigTests(unittest.TestCase): diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 06004e258b2eff..a94924e30db187 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -63,6 +63,18 @@ static datetime_state _datetime_global_state; #define STATIC_STATE() (&_datetime_global_state) +void +set_datetime_capi_by_interp(PyDateTime_CAPI *capi) +{ + _PyInterpreterState_GET()->datetime_capi = capi; +} + +PyDateTime_CAPI * +get_datetime_capi_by_interp(void) +{ + return (PyDateTime_CAPI *)_PyInterpreterState_GET()->datetime_capi; +} + /* We require that C int be at least 32 bits, and use int virtually * everywhere. In just a few cases we use a temp long, where a Python * API returns a C long. In such cases, we have to ensure that the @@ -6736,12 +6748,15 @@ get_datetime_capi(void) datetime_state *st = STATIC_STATE(); assert(st->utc != NULL); capi->TimeZone_UTC = st->utc; // borrowed ref + capi->_set_capi_by_interp = set_datetime_capi_by_interp; + capi->_get_capi_by_interp = get_datetime_capi_by_interp; return capi; } static void datetime_destructor(PyObject *op) { + set_datetime_capi_by_interp(NULL); void *ptr = PyCapsule_GetPointer(op, PyDateTime_CAPSULE_NAME); PyMem_Free(ptr); } diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 21c5f696a4f2ec..1d25919ff3b511 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -952,3 +952,38 @@ PyInit__test_shared_gil_only(void) { return PyModuleDef_Init(&shared_gil_only_def); } + + +#include "datetime.h" + +static int +datetime_capi_client_exec(PyObject *m) +{ + _get_pydatetime_capi = _get_pydatetime_capi_dummy; + if (PyDateTimeAPI != NULL) { + return -1; + } + PyDateTime_IMPORT; + if (PyDateTimeAPI == NULL) { + return -1; + } + if (PyDateTimeAPI != PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0)) { + return -1; + } + return 0; +} + +static PyModuleDef_Slot datetime_capi_client_slots[] = { + {Py_mod_exec, datetime_capi_client_exec}, + {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED}, + {0, NULL}, +}; + +static PyModuleDef datetime_capi_client_def = TEST_MODULE_DEF( + "_testmultiphase_datetime_capi_client", datetime_capi_client_slots, NULL); + +PyMODINIT_FUNC +PyInit__test_datetime_capi_client(void) +{ + return PyModuleDef_Init(&datetime_capi_client_def); +} From fecd19228ac43eb7a076cb5a04cbaed92a336fa4 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sun, 28 Apr 2024 08:34:22 +0900 Subject: [PATCH 02/18] globals-to-fix.tsv --- Include/datetime.h | 1 + Tools/c-analyzer/cpython/globals-to-fix.tsv | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Include/datetime.h b/Include/datetime.h index abb2295b17099c..b6b96ca433eafc 100644 --- a/Include/datetime.h +++ b/Include/datetime.h @@ -182,6 +182,7 @@ typedef struct _pydatetime_capi { PyObject *(*DateTime_FromDateAndTimeAndFold)(int, int, int, int, int, int, int, PyObject*, int, PyTypeObject*); PyObject *(*Time_FromTimeAndFold)(int, int, int, int, PyObject*, int, PyTypeObject*); + struct _pydatetime_capi *(*_get_capi_by_interp)(void); void (*_set_capi_by_interp)(struct _pydatetime_capi *); } PyDateTime_CAPI; diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index 79a8f850ec1451..604b05d40e0436 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -446,7 +446,7 @@ Modules/_tkinter.c - trbInCmd - ## initialized once ## other -Include/datetime.h - PyDateTimeAPI - +Include/datetime.h - _get_pydatetime_capi - Modules/_ctypes/cfield.c _ctypes_get_fielddesc initialized - Modules/_ctypes/malloc_closure.c - _pagesize - Modules/_cursesmodule.c - initialised - From 2bdf01cc3f54de901823eea8b0be79a48edaf309 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sun, 28 Apr 2024 09:08:14 +0900 Subject: [PATCH 03/18] make exit code individual --- Modules/_testmultiphase.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 1d25919ff3b511..fa0caa30f7770e 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -965,10 +965,10 @@ datetime_capi_client_exec(PyObject *m) } PyDateTime_IMPORT; if (PyDateTimeAPI == NULL) { - return -1; + return -2; } if (PyDateTimeAPI != PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0)) { - return -1; + return -3; } return 0; } From bead43116ed999766545a5a7f09394e18c1d8926 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sun, 28 Apr 2024 10:57:09 +0900 Subject: [PATCH 04/18] fix gil-disabled test --- Modules/_testmultiphase.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index fa0caa30f7770e..9e16b7f039b838 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -964,12 +964,11 @@ datetime_capi_client_exec(PyObject *m) return -1; } PyDateTime_IMPORT; - if (PyDateTimeAPI == NULL) { - return -2; - } + PyErr_Clear(); if (PyDateTimeAPI != PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0)) { - return -3; + return -1; } + PyErr_Clear(); return 0; } From d361ee48db5309e61e883226ef19b9ef412cb2be Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Mon, 29 Apr 2024 23:14:25 +0900 Subject: [PATCH 05/18] add an assertion in datetime_destructor() --- Modules/_datetimemodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index a94924e30db187..3b08f147ad6606 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -6756,9 +6756,10 @@ get_datetime_capi(void) static void datetime_destructor(PyObject *op) { - set_datetime_capi_by_interp(NULL); void *ptr = PyCapsule_GetPointer(op, PyDateTime_CAPSULE_NAME); + assert(ptr == get_datetime_capi_by_interp()); PyMem_Free(ptr); + set_datetime_capi_by_interp(NULL); } static int From 3b721bc0331110e66ae65dda49afc44b92ac280e Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Mon, 29 Apr 2024 23:35:11 +0900 Subject: [PATCH 06/18] fix assert() --- Modules/_datetimemodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 3b08f147ad6606..a60c62e339f248 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -6757,7 +6757,7 @@ static void datetime_destructor(PyObject *op) { void *ptr = PyCapsule_GetPointer(op, PyDateTime_CAPSULE_NAME); - assert(ptr == get_datetime_capi_by_interp()); + assert(!get_datetime_capi_by_interp() || ptr == get_datetime_capi_by_interp()); PyMem_Free(ptr); set_datetime_capi_by_interp(NULL); } From 53b0219e2e7b60ac87156ed7c680fa582c34c477 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 30 Apr 2024 02:59:10 +0900 Subject: [PATCH 07/18] add a struct for internal use --- Include/datetime.h | 14 +++++++++----- Modules/_datetimemodule.c | 6 +++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Include/datetime.h b/Include/datetime.h index b6b96ca433eafc..40211c26ecaec2 100644 --- a/Include/datetime.h +++ b/Include/datetime.h @@ -155,7 +155,7 @@ typedef struct /* Define structure for C API. */ -typedef struct _pydatetime_capi { +typedef struct { /* type objects */ PyTypeObject *DateType; PyTypeObject *DateTimeType; @@ -183,12 +183,16 @@ typedef struct _pydatetime_capi { PyObject*, int, PyTypeObject*); PyObject *(*Time_FromTimeAndFold)(int, int, int, int, PyObject*, int, PyTypeObject*); - struct _pydatetime_capi *(*_get_capi_by_interp)(void); - void (*_set_capi_by_interp)(struct _pydatetime_capi *); } PyDateTime_CAPI; #define PyDateTime_CAPSULE_NAME "datetime.datetime_CAPI" +/* Internal use */ +typedef struct { + PyDateTime_CAPI api; + PyDateTime_CAPI *(*_get_capi_by_interp)(void); + void (*_set_capi_by_interp)(PyDateTime_CAPI *); +} _pydatetime_capi; /* This block is only used as part of the public API and should not be * included in _datetimemodule.c, which does not use the C API capsule. @@ -205,9 +209,9 @@ static PyDateTime_CAPI *(*_get_pydatetime_capi)(void) = _get_pydatetime_capi_dum #define PyDateTimeAPI _get_pydatetime_capi() static inline void pydatetime_import(void) { - PyDateTime_CAPI *capi = PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0); + _pydatetime_capi *capi = PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0); if (capi) { - capi->_set_capi_by_interp(capi); + capi->_set_capi_by_interp((PyDateTime_CAPI *)capi); _get_pydatetime_capi = capi->_get_capi_by_interp; } } diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index a60c62e339f248..a3ba202b4539d7 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -6724,7 +6724,7 @@ static PyMethodDef module_methods[] = { static inline PyDateTime_CAPI * get_datetime_capi(void) { - PyDateTime_CAPI *capi = PyMem_Malloc(sizeof(PyDateTime_CAPI)); + PyDateTime_CAPI *capi = PyMem_Malloc(sizeof(_pydatetime_capi)); if (capi == NULL) { PyErr_NoMemory(); return NULL; @@ -6748,8 +6748,8 @@ get_datetime_capi(void) datetime_state *st = STATIC_STATE(); assert(st->utc != NULL); capi->TimeZone_UTC = st->utc; // borrowed ref - capi->_set_capi_by_interp = set_datetime_capi_by_interp; - capi->_get_capi_by_interp = get_datetime_capi_by_interp; + ((_pydatetime_capi *)capi)->_set_capi_by_interp = set_datetime_capi_by_interp; + ((_pydatetime_capi *)capi)->_get_capi_by_interp = get_datetime_capi_by_interp; return capi; } From 8b10d7002762b8bac01aa63f2b9a559b62bf40b9 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 30 Apr 2024 21:24:02 +0900 Subject: [PATCH 08/18] try hiding internal info --- Include/datetime.h | 29 +++++---------------- Modules/_datetimemodule.c | 29 ++++++++++++++++----- Modules/_testcapi/datetime.c | 3 +++ Modules/_testmultiphase.c | 2 +- Tools/c-analyzer/cpython/globals-to-fix.tsv | 1 - 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Include/datetime.h b/Include/datetime.h index 40211c26ecaec2..b023da24be35f6 100644 --- a/Include/datetime.h +++ b/Include/datetime.h @@ -187,35 +187,18 @@ typedef struct { #define PyDateTime_CAPSULE_NAME "datetime.datetime_CAPI" -/* Internal use */ -typedef struct { - PyDateTime_CAPI api; - PyDateTime_CAPI *(*_get_capi_by_interp)(void); - void (*_set_capi_by_interp)(PyDateTime_CAPI *); -} _pydatetime_capi; +PyAPI_FUNC(void) _PyDateTimeAPI_Import(void); +PyAPI_FUNC(void) _PyDateTimeAPI_Clear(void); +PyAPI_FUNC(PyDateTime_CAPI *) _PyDateTimeAPI_Get(void); /* This block is only used as part of the public API and should not be * included in _datetimemodule.c, which does not use the C API capsule. * See bpo-35081 for more details. * */ #ifndef _PY_DATETIME_IMPL -/* Define global variable for the C API and a macro for setting it. */ -static PyDateTime_CAPI * -_get_pydatetime_capi_dummy(void) -{ - return NULL; -} -static PyDateTime_CAPI *(*_get_pydatetime_capi)(void) = _get_pydatetime_capi_dummy; -#define PyDateTimeAPI _get_pydatetime_capi() - -static inline void pydatetime_import(void) { - _pydatetime_capi *capi = PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0); - if (capi) { - capi->_set_capi_by_interp((PyDateTime_CAPI *)capi); - _get_pydatetime_capi = capi->_get_capi_by_interp; - } -} -#define PyDateTime_IMPORT pydatetime_import() + +#define PyDateTimeAPI _PyDateTimeAPI_Get() +#define PyDateTime_IMPORT _PyDateTimeAPI_Import() /* Macro for access to the UTC singleton */ #define PyDateTime_TimeZone_UTC PyDateTimeAPI->TimeZone_UTC diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index a3ba202b4539d7..ecccf93e32ab00 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -63,18 +63,35 @@ static datetime_state _datetime_global_state; #define STATIC_STATE() (&_datetime_global_state) -void +static inline void set_datetime_capi_by_interp(PyDateTime_CAPI *capi) { _PyInterpreterState_GET()->datetime_capi = capi; } +void +_PyDateTimeAPI_Import(void) +{ + PyDateTime_CAPI *capi = PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0); + if (capi) { + // PyInit__datetime() is not called when the module is already loaded + // with single-phase init. + set_datetime_capi_by_interp((PyDateTime_CAPI *)capi); + } +} + PyDateTime_CAPI * -get_datetime_capi_by_interp(void) +_PyDateTimeAPI_Get(void) { return (PyDateTime_CAPI *)_PyInterpreterState_GET()->datetime_capi; } +void +_PyDateTimeAPI_Clear(void) +{ + set_datetime_capi_by_interp(NULL); +} + /* We require that C int be at least 32 bits, and use int virtually * everywhere. In just a few cases we use a temp long, where a Python * API returns a C long. In such cases, we have to ensure that the @@ -6724,7 +6741,7 @@ static PyMethodDef module_methods[] = { static inline PyDateTime_CAPI * get_datetime_capi(void) { - PyDateTime_CAPI *capi = PyMem_Malloc(sizeof(_pydatetime_capi)); + PyDateTime_CAPI *capi = PyMem_Malloc(sizeof(PyDateTime_CAPI)); if (capi == NULL) { PyErr_NoMemory(); return NULL; @@ -6748,8 +6765,6 @@ get_datetime_capi(void) datetime_state *st = STATIC_STATE(); assert(st->utc != NULL); capi->TimeZone_UTC = st->utc; // borrowed ref - ((_pydatetime_capi *)capi)->_set_capi_by_interp = set_datetime_capi_by_interp; - ((_pydatetime_capi *)capi)->_get_capi_by_interp = get_datetime_capi_by_interp; return capi; } @@ -6757,9 +6772,7 @@ static void datetime_destructor(PyObject *op) { void *ptr = PyCapsule_GetPointer(op, PyDateTime_CAPSULE_NAME); - assert(!get_datetime_capi_by_interp() || ptr == get_datetime_capi_by_interp()); PyMem_Free(ptr); - set_datetime_capi_by_interp(NULL); } static int @@ -6959,6 +6972,8 @@ _datetime_exec(PyObject *module) PyMem_Free(capi); goto error; } + /* Ensure that the newest capi is used on multi-phase init */ + set_datetime_capi_by_interp(capi); /* A 4-year cycle has an extra leap day over what we'd get from * pasting together 4 single years. diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index b1796039f0d83a..7050f492bfa745 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -8,6 +8,9 @@ static int test_run_counter = 0; static PyObject * test_datetime_capi(PyObject *self, PyObject *args) { + if (!test_run_counter) { + _PyDateTimeAPI_Clear(); + } if (PyDateTimeAPI) { if (test_run_counter) { /* Probably regrtest.py -R */ diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 9e16b7f039b838..29114b75d99385 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -959,7 +959,7 @@ PyInit__test_shared_gil_only(void) static int datetime_capi_client_exec(PyObject *m) { - _get_pydatetime_capi = _get_pydatetime_capi_dummy; + _PyDateTimeAPI_Clear(); if (PyDateTimeAPI != NULL) { return -1; } diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index 604b05d40e0436..ae6a579dae1b23 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -446,7 +446,6 @@ Modules/_tkinter.c - trbInCmd - ## initialized once ## other -Include/datetime.h - _get_pydatetime_capi - Modules/_ctypes/cfield.c _ctypes_get_fielddesc initialized - Modules/_ctypes/malloc_closure.c - _pagesize - Modules/_cursesmodule.c - initialised - From fb0f5efdf95cf56b288ce70bfddac18addf22fda Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 04:40:27 +0900 Subject: [PATCH 09/18] apply suggested changes --- Lib/test/test_capi/test_misc.py | 2 +- Modules/_datetimemodule.c | 2 +- Modules/_testmultiphase.c | 49 +++++++++++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 98849b8d179b83..9786e0d88cbf66 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2294,7 +2294,7 @@ def test_datetime_capi_client(self): module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) """) - exec(script) + exec(script) # run main interp first ret = support.run_in_subinterp(script) self.assertEqual(ret, 0) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index ecccf93e32ab00..356956144d9865 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -76,7 +76,7 @@ _PyDateTimeAPI_Import(void) if (capi) { // PyInit__datetime() is not called when the module is already loaded // with single-phase init. - set_datetime_capi_by_interp((PyDateTime_CAPI *)capi); + set_datetime_capi_by_interp(capi); } } diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 29114b75d99385..2a7a79c15170b7 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -956,16 +956,59 @@ PyInit__test_shared_gil_only(void) #include "datetime.h" +static int +datetime_capi_import_with_error(void) +{ + static int multiphase = -1; + int ismain = PyInterpreterState_Get() == PyInterpreterState_Main(); + if (ismain) { + PyObject *module = PyImport_ImportModule("_datetime"); + if (module == NULL) { + return -1; + } + PyModuleDef *def = PyModule_GetDef(module); + Py_DECREF(module); + if (def && def->m_size >= 0) { + multiphase = 1; + } + else { + multiphase = 0; + } + } + if (multiphase < 0) { + PyErr_SetString(PyExc_AssertionError, + "Main interpreter must be loaded first."); + return -1; + } + + _PyDateTimeAPI_Import(); + if (!PyErr_Occurred()) { + return 0; + } +#ifdef Py_GIL_DISABLED + if (!ismain && !multiphase) { + PyErr_WriteUnraisable(NULL); + return 0; + } +#endif + return -1; +} + static int datetime_capi_client_exec(PyObject *m) { _PyDateTimeAPI_Clear(); - if (PyDateTimeAPI != NULL) { + if (_PyDateTimeAPI_Get() != NULL) { + PyErr_SetString(PyExc_AssertionError, + "DateTime API is expected to remain NULL."); + return -1; + } + if (datetime_capi_import_with_error() < 0) { return -1; } - PyDateTime_IMPORT; - PyErr_Clear(); if (PyDateTimeAPI != PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0)) { + PyErr_SetString(PyExc_AssertionError, + "DateTime API does not match Capsule CAPI."); return -1; } PyErr_Clear(); From 7de45acc8b3211a406e88acbb06a0a5d6774a908 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 05:34:47 +0900 Subject: [PATCH 10/18] ignored.tsv --- Modules/_testmultiphase.c | 10 +++++----- Tools/c-analyzer/cpython/ignored.tsv | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 2a7a79c15170b7..b8f289d5150552 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -959,7 +959,7 @@ PyInit__test_shared_gil_only(void) static int datetime_capi_import_with_error(void) { - static int multiphase = -1; + static int is_datetime_multiphase = -1; int ismain = PyInterpreterState_Get() == PyInterpreterState_Main(); if (ismain) { PyObject *module = PyImport_ImportModule("_datetime"); @@ -969,13 +969,13 @@ datetime_capi_import_with_error(void) PyModuleDef *def = PyModule_GetDef(module); Py_DECREF(module); if (def && def->m_size >= 0) { - multiphase = 1; + is_datetime_multiphase = 1; } else { - multiphase = 0; + is_datetime_multiphase = 0; } } - if (multiphase < 0) { + if (is_datetime_multiphase < 0) { PyErr_SetString(PyExc_AssertionError, "Main interpreter must be loaded first."); return -1; @@ -986,7 +986,7 @@ datetime_capi_import_with_error(void) return 0; } #ifdef Py_GIL_DISABLED - if (!ismain && !multiphase) { + if (!ismain && !is_datetime_multiphase) { PyErr_WriteUnraisable(NULL); return 0; } diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 87b695de23e25e..e87f91a45272dd 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -576,6 +576,7 @@ Modules/_testmultiphase.c - def_nonmodule_with_methods - Modules/_testmultiphase.c - main_def - Modules/_testmultiphase.c - main_slots - Modules/_testmultiphase.c - meth_state_access_slots - +Modules/_testmultiphase.c - is_datetime_multiphase - Modules/_testmultiphase.c - nonmodule_methods - Modules/_testmultiphase.c - null_slots_def - Modules/_testmultiphase.c - slots_bad_large - From f412c55577e3aab0a030f51e21439179f1275c32 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 05:56:12 +0900 Subject: [PATCH 11/18] globals-to-fix.tsv --- Tools/c-analyzer/cpython/globals-to-fix.tsv | 1 + Tools/c-analyzer/cpython/ignored.tsv | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index ae6a579dae1b23..9f6cf520af261a 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -456,6 +456,7 @@ Modules/_elementtree.c - expat_capi - Modules/readline.c - libedit_append_replace_history_offset - Modules/readline.c - using_libedit_emulation - Modules/readline.c - libedit_history_start - +Modules/_testmultiphase.c - is_datetime_multiphase - ##----------------------- ## state diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index e87f91a45272dd..87b695de23e25e 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -576,7 +576,6 @@ Modules/_testmultiphase.c - def_nonmodule_with_methods - Modules/_testmultiphase.c - main_def - Modules/_testmultiphase.c - main_slots - Modules/_testmultiphase.c - meth_state_access_slots - -Modules/_testmultiphase.c - is_datetime_multiphase - Modules/_testmultiphase.c - nonmodule_methods - Modules/_testmultiphase.c - null_slots_def - Modules/_testmultiphase.c - slots_bad_large - From c00ed1f71d261138f2d02aaf401a86976a44669a Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 06:26:18 +0900 Subject: [PATCH 12/18] globals-to-fix.tsv --- Tools/c-analyzer/cpython/globals-to-fix.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index 9f6cf520af261a..bf950f95072751 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -456,7 +456,7 @@ Modules/_elementtree.c - expat_capi - Modules/readline.c - libedit_append_replace_history_offset - Modules/readline.c - using_libedit_emulation - Modules/readline.c - libedit_history_start - -Modules/_testmultiphase.c - is_datetime_multiphase - +Modules/_testmultiphase.c datetime_capi_import_with_error is_datetime_multiphase - ##----------------------- ## state From e4986f5a13907d3b892e4be5384baa2ed5ee92de Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 06:35:12 +0900 Subject: [PATCH 13/18] ignored.tsv again --- Tools/c-analyzer/cpython/globals-to-fix.tsv | 1 - Tools/c-analyzer/cpython/ignored.tsv | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index bf950f95072751..ae6a579dae1b23 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -456,7 +456,6 @@ Modules/_elementtree.c - expat_capi - Modules/readline.c - libedit_append_replace_history_offset - Modules/readline.c - using_libedit_emulation - Modules/readline.c - libedit_history_start - -Modules/_testmultiphase.c datetime_capi_import_with_error is_datetime_multiphase - ##----------------------- ## state diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 87b695de23e25e..11e56658951af8 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -591,6 +591,7 @@ Modules/_testmultiphase.c - slots_exec_unreported_exception - Modules/_testmultiphase.c - slots_nonmodule_with_exec_slots - Modules/_testmultiphase.c - testexport_methods - Modules/_testmultiphase.c - uninitialized_def - +Modules/_testmultiphase.c datetime_capi_import_with_error is_datetime_multiphase - Modules/_testsinglephase.c - global_state - Modules/_xxtestfuzz/_xxtestfuzz.c - _fuzzmodule - Modules/_xxtestfuzz/_xxtestfuzz.c - module_methods - From 39e17c72359fb53cbdf17e7188480a37ce6fecb8 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 06:53:25 +0900 Subject: [PATCH 14/18] run main interp twice in test --- Lib/test/test_capi/test_misc.py | 1 + Modules/_testmultiphase.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 9786e0d88cbf66..2cbaac346e915c 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2295,6 +2295,7 @@ def test_datetime_capi_client(self): spec.loader.exec_module(module) """) exec(script) # run main interp first + exec(script) # run main interp twice ret = support.run_in_subinterp(script) self.assertEqual(ret, 0) diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index b8f289d5150552..ba19b8b14f73af 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -961,7 +961,7 @@ datetime_capi_import_with_error(void) { static int is_datetime_multiphase = -1; int ismain = PyInterpreterState_Get() == PyInterpreterState_Main(); - if (ismain) { + if (ismain && is_datetime_multiphase < 0) { PyObject *module = PyImport_ImportModule("_datetime"); if (module == NULL) { return -1; From 7aa369eccc5ef475ba5b73b8ecbb4a6ee9ba9c79 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 07:08:37 +0900 Subject: [PATCH 15/18] add a comment to a test --- Modules/_testmultiphase.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index ba19b8b14f73af..6860fbddb59df4 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -987,6 +987,7 @@ datetime_capi_import_with_error(void) } #ifdef Py_GIL_DISABLED if (!ismain && !is_datetime_multiphase) { + // _datetime module and Capsule are not imported PyErr_WriteUnraisable(NULL); return 0; } From 0769c965cf6de74e1f947ffcda2533fb6b408aee Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 18:41:04 +0900 Subject: [PATCH 16/18] add an internal capsule --- Include/datetime.h | 22 ++++++++++--- Lib/test/datetimetester.py | 2 +- Modules/_datetimemodule.c | 34 +++++++++++++-------- Modules/_testcapi/datetime.c | 3 -- Modules/_testmultiphase.c | 2 +- Tools/c-analyzer/cpython/globals-to-fix.tsv | 1 + 6 files changed, 42 insertions(+), 22 deletions(-) diff --git a/Include/datetime.h b/Include/datetime.h index b023da24be35f6..cb79b1880fb756 100644 --- a/Include/datetime.h +++ b/Include/datetime.h @@ -186,16 +186,30 @@ typedef struct { } PyDateTime_CAPI; #define PyDateTime_CAPSULE_NAME "datetime.datetime_CAPI" - -PyAPI_FUNC(void) _PyDateTimeAPI_Import(void); -PyAPI_FUNC(void) _PyDateTimeAPI_Clear(void); -PyAPI_FUNC(PyDateTime_CAPI *) _PyDateTimeAPI_Get(void); +#define PyDateTime_INTERNAL_CAPSULE_NAME "datetime.datetime_CAPI_INTERNAL" /* This block is only used as part of the public API and should not be * included in _datetimemodule.c, which does not use the C API capsule. * See bpo-35081 for more details. * */ #ifndef _PY_DATETIME_IMPL +static PyDateTime_CAPI * +_PyDateTimeAPI_not_ready(void) +{ + return NULL; +} +typedef PyDateTime_CAPI *(*datetime_api_getfunc)(void); +static datetime_api_getfunc _PyDateTimeAPI_Get = _PyDateTimeAPI_not_ready; + +static inline void +_PyDateTimeAPI_Import(void) +{ + datetime_api_getfunc (*func)(void) = PyCapsule_Import( + PyDateTime_INTERNAL_CAPSULE_NAME, 0); + if (func) { + _PyDateTimeAPI_Get = func(); + } +} #define PyDateTimeAPI _PyDateTimeAPI_Get() #define PyDateTime_IMPORT _PyDateTimeAPI_Import() diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 570110893629cf..6946b394e68a82 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -91,7 +91,7 @@ def test_name_cleanup(self): if not name.startswith('__') and not name.endswith('__')) allowed = set(['MAXYEAR', 'MINYEAR', 'date', 'datetime', 'datetime_CAPI', 'time', 'timedelta', 'timezone', - 'tzinfo', 'UTC', 'sys']) + 'tzinfo', 'UTC', 'sys', 'datetime_CAPI_INTERNAL']) self.assertEqual(names - allowed, set([])) def test_divide_and_round(self): diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 356956144d9865..16ec3bd61144e0 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -69,7 +69,13 @@ set_datetime_capi_by_interp(PyDateTime_CAPI *capi) _PyInterpreterState_GET()->datetime_capi = capi; } -void +static PyDateTime_CAPI * +_PyDateTimeAPI_Get(void) +{ + return (PyDateTime_CAPI *)_PyInterpreterState_GET()->datetime_capi; +} + +static void * _PyDateTimeAPI_Import(void) { PyDateTime_CAPI *capi = PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0); @@ -77,19 +83,9 @@ _PyDateTimeAPI_Import(void) // PyInit__datetime() is not called when the module is already loaded // with single-phase init. set_datetime_capi_by_interp(capi); + return _PyDateTimeAPI_Get; } -} - -PyDateTime_CAPI * -_PyDateTimeAPI_Get(void) -{ - return (PyDateTime_CAPI *)_PyInterpreterState_GET()->datetime_capi; -} - -void -_PyDateTimeAPI_Clear(void) -{ - set_datetime_capi_by_interp(NULL); + return NULL; } /* We require that C int be at least 32 bits, and use int virtually @@ -6972,6 +6968,18 @@ _datetime_exec(PyObject *module) PyMem_Free(capi); goto error; } + + capsule = PyCapsule_New(_PyDateTimeAPI_Import, + PyDateTime_INTERNAL_CAPSULE_NAME, NULL); + if (capsule == NULL) { + PyMem_Free(capi); + goto error; + } + if (PyModule_Add(module, "datetime_CAPI_INTERNAL", capsule) < 0) { + PyMem_Free(capi); + goto error; + } + /* Ensure that the newest capi is used on multi-phase init */ set_datetime_capi_by_interp(capi); diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index 7050f492bfa745..b1796039f0d83a 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -8,9 +8,6 @@ static int test_run_counter = 0; static PyObject * test_datetime_capi(PyObject *self, PyObject *args) { - if (!test_run_counter) { - _PyDateTimeAPI_Clear(); - } if (PyDateTimeAPI) { if (test_run_counter) { /* Probably regrtest.py -R */ diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 6860fbddb59df4..429824fbede1fa 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -998,7 +998,7 @@ datetime_capi_import_with_error(void) static int datetime_capi_client_exec(PyObject *m) { - _PyDateTimeAPI_Clear(); + _PyDateTimeAPI_Get = _PyDateTimeAPI_not_ready; if (_PyDateTimeAPI_Get() != NULL) { PyErr_SetString(PyExc_AssertionError, "DateTime API is expected to remain NULL."); diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index ae6a579dae1b23..3c0f3b39530d3f 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -446,6 +446,7 @@ Modules/_tkinter.c - trbInCmd - ## initialized once ## other +Include/datetime.h - _PyDateTimeAPI_Get - Modules/_ctypes/cfield.c _ctypes_get_fielddesc initialized - Modules/_ctypes/malloc_closure.c - _pagesize - Modules/_cursesmodule.c - initialised - From de31431d3c6560fb187dd11df5f66905a841515f Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 1 May 2024 19:19:45 +0900 Subject: [PATCH 17/18] remove typedef --- Include/datetime.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Include/datetime.h b/Include/datetime.h index cb79b1880fb756..54d80303a281ce 100644 --- a/Include/datetime.h +++ b/Include/datetime.h @@ -198,14 +198,12 @@ _PyDateTimeAPI_not_ready(void) { return NULL; } -typedef PyDateTime_CAPI *(*datetime_api_getfunc)(void); -static datetime_api_getfunc _PyDateTimeAPI_Get = _PyDateTimeAPI_not_ready; +static PyDateTime_CAPI *(*_PyDateTimeAPI_Get)(void) = _PyDateTimeAPI_not_ready; static inline void _PyDateTimeAPI_Import(void) { - datetime_api_getfunc (*func)(void) = PyCapsule_Import( - PyDateTime_INTERNAL_CAPSULE_NAME, 0); + void *(*func)(void) = PyCapsule_Import(PyDateTime_INTERNAL_CAPSULE_NAME, 0); if (func) { _PyDateTimeAPI_Get = func(); } From 8dadc4929fb23602ac51aa932fb50f3bb42f4a24 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Mon, 6 May 2024 00:54:13 +0900 Subject: [PATCH 18/18] add Py_MOD_GIL_NOT_USED to the test --- Modules/_testmultiphase.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 4bb372058076aa..9d5263482360e3 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -994,7 +994,7 @@ datetime_capi_import_with_error(void) } if (is_datetime_multiphase < 0) { PyErr_SetString(PyExc_AssertionError, - "Main interpreter must be loaded first."); + "Main interpreter must be tested first."); return -1; } @@ -1036,6 +1036,7 @@ datetime_capi_client_exec(PyObject *m) static PyModuleDef_Slot datetime_capi_client_slots[] = { {Py_mod_exec, datetime_capi_client_exec}, {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED}, + {Py_mod_gil, Py_MOD_GIL_NOT_USED}, {0, NULL}, };