Skip to content

gh-151039: Fix a crash when _datetimetypes outlive _datetime module#151044

Open
sobolevn wants to merge 6 commits into
python:mainfrom
sobolevn:issue-151039
Open

gh-151039: Fix a crash when _datetimetypes outlive _datetime module#151044
sobolevn wants to merge 6 commits into
python:mainfrom
sobolevn:issue-151039

Conversation

@sobolevn
Copy link
Copy Markdown
Member

@sobolevn sobolevn commented Jun 7, 2026

Several changes:

  1. Always check that GET_CURRENT_STATE can return NULL
  2. Unwrap (void)PyWeakref_GetRef call into a full check for safety, there are multiple things that can go wrong when getting a weakref
  3. Remove assert(!PyErr_Occurred()); followed by if (PyErr_Occurred()) {

@sobolevn sobolevn requested a review from picnixz June 7, 2026 14:33
@sobolevn sobolevn added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jun 7, 2026
@sobolevn
Copy link
Copy Markdown
Member Author

sobolevn commented Jun 7, 2026

Ok, I just discovered #132413 which is related.

Comment thread Lib/test/datetimetester.py Outdated
Comment thread Misc/NEWS.d/next/Library/2026-06-07-17-29-33.gh-issue-151039.AZ0qBn.rst Outdated
…Z0qBn.rst

Co-authored-by: Stan Ulbrych <stan@python.org>
Comment thread Lib/test/datetimetester.py Outdated
Comment thread Lib/test/datetimetester.py Outdated
Comment thread Modules/_datetimemodule.c Outdated
@sobolevn sobolevn changed the title gh-151039: Fix a crash when datetime.timedelta outlives _datetime module gh-151039: Fix a crash when _datetimetypes outlive _datetime module Jun 8, 2026
@vstinner
Copy link
Copy Markdown
Member

vstinner commented Jun 8, 2026

Can you change get_current_module() to return an integer? -1 on error, 0 if the module is NULL, 1 on success (module is not NULL). It would avoid having to call PyErr_Occurred() at each call. Example of patch:

Details
diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c
index eb05539d4c2..efc81538e38 100644
--- a/Modules/_datetimemodule.c
+++ b/Modules/_datetimemodule.c
@@ -126,8 +126,8 @@ get_module_state(PyObject *module)
 
 #define INTERP_KEY ((PyObject *)&_Py_ID(cached_datetime_module))
 
-static PyObject *
-get_current_module(PyInterpreterState *interp)
+static int
+get_current_module(PyInterpreterState *interp, PyObject **p_mod)
 {
     PyObject *mod = NULL;
 
@@ -151,11 +151,14 @@ get_current_module(PyInterpreterState *interp)
             Py_DECREF(ref);
         }
     }
-    return mod;
+    *p_mod = mod;
+    assert(!PyErr_Occurred());
+    return (mod != NULL);
 
 error:
     assert(PyErr_Occurred());
-    return NULL;
+    *p_mod = mod;
+    return -1;
 }
 
 static PyModuleDef datetimemodule;
@@ -164,11 +167,11 @@ static datetime_state *
 _get_current_state(PyObject **p_mod)
 {
     PyInterpreterState *interp = PyInterpreterState_Get();
-    PyObject *mod = get_current_module(interp);
+    PyObject *mod;
+    if (get_current_module(interp, &mod) < 0) {
+        goto error;
+    }
     if (mod == NULL) {
-        if (PyErr_Occurred()) {
-            goto error;
-        }
         /* The static types can outlive the module,
          * so we must re-import the module. */
         mod = PyImport_ImportModule("_datetime");
@@ -7610,8 +7613,8 @@ _datetime_exec(PyObject *module)
     datetime_state *st = get_module_state(module);
 
     PyInterpreterState *interp = PyInterpreterState_Get();
-    PyObject *old_module = get_current_module(interp);
-    if (PyErr_Occurred()) {
+    PyObject *old_module;
+    if (get_current_module(interp, &old_module) < 0) {
         assert(old_module == NULL);
         goto error;
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants