10000 gh-117398: Add datetime Module State by ericsnowcurrently · Pull Request #119810 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-117398: Add datetime Module State #119810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Jun 3, 2024
Merged

gh-117398: Add datetime Module State #119810

Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8463ae8
Use the static types directly.
ericsnowcurrently May 28, 2024
ead0083
Use the UTC singleton directly.
ericsnowcurrently May 28, 2024
4c52d5d
Add module state.
ericsnowcurrently May 27, 2024
6b291c8
Clear the module state when destroying the module.
ericsnowcurrently May 28, 2024
69c7e1f
Create PyDateTime_IsoCalendarDateType in init_state().
ericsnowcurrently May 29, 2024
36dbeea
Track the "current" module in the internal per-interpreter dict.
ericsnowcurrently May 29, 2024
d0c0b2d
Copy the "current" module state, if available.
ericsnowcurrently May 29, 2024
0599dd6
Make state usage explicit.
ericsnowcurrently May 29, 2024
1f1f6fc
Reduce churn.
ericsnowcurrently May 29, 2024
d44d6e9
Drop _datetime_global_state.
ericsnowcurrently May 29, 2024
a309474
Drop datetime_state.initialized.
ericsnowcurrently May 29, 2024
ede4415
Fix refleaks.
ericsnowcurrently May 30, 2024
5a8b1aa
Clear the "current" module when finalizing the module.
ericsnowcurrently May 30, 2024
3de1cd3
Use a weakref when tracking the "current" module.
ericsnowcurrently May 30, 2024
62b3d5e
Fix clear_current_module().
ericsnowcurrently May 30, 2024
5c25927
Give each module its own heap types.
ericsnowcurrently May 31, 2024
da24674
Consolidate into init_state().
ericsnowcurrently May 31, 2024
f8420ea 8000
Use _Py_ID() for INTERP_KEY.
ericsnowcurrently May 31, 2024
2a2c0b1
Handle PyWeakref_GetRef() returning None.
ericsnowcurrently May 31, 2024
c519e3c
Fix refleak.
ericsnowcurrently May 31, 2024
05acc56
Make sure the module exists in static type methods.
ericsnowcurrently May 31, 2024
07e3b65
Don't bother making the module temporary.
ericsnowcurrently May 31, 2024
dc4b458
Add a comment about the macros.
ericsnowcurrently Jun 3, 2024
3c55035
Simplify GET_CURRENT_STATE().
ericsnowcurrently Jun 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix refleaks.
  • Loading branch information
@ericsnowcurrently
ericsnowcurrently committed May 30, 2024
commit ede4415c638334fa64b3b695c5f08a431f274b4c
8 changes: 7 additions & 1 deletion Modules/_datetimemodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7059,8 +7059,12 @@ _datetime_exec(PyObject *module)
}
}

/* For now we only set the objects on the static types once.
* We will relax that once each types __dict__ is per-interpreter. */
#define DATETIME_ADD_MACRO(dict, c, value_expr) \
do { \
if (PyDict_GetItemString(dict, c) == NULL) { \
Comment on lines 7150 to +7152
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This macro needs Py_DECREF(value_expr) if the condition is not true.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expression isn't evaluated until the second statement inside this branch. Perhaps I've missed something?

Copy link
Contributor
@neonene neonene May 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

#define DATETIME_ADD_MACRO(dict, c, value_expr)         \
    do {                                                \
      if (PyDict_GetItemString(dict, c) == NULL) {      \
        ...                                             \
        Py_DECREF(value);                               \
      }                                                 \
+     else {                                            \
+       Py_DECREF(value_expr);                          \
+     }                                                 \
    } while(0)

If this example fixes the refleaks test, then the leak comes from:

cpython/Modules/_datetimemodule.c

Lines 7173 to 7179 in 3de1cd3

/* -23:59 */
PyObject *min = create_timezone_from_delta(-1, 60, 0, 1);
DATETIME_ADD_MACRO(d, "min", min);
/* +23:59 */
PyObject *max = create_timezone_from_delta(0, (23 * 60 + 59) * 60, 0, 0);
DATETIME_ADD_MACRO(d, "max", max);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I understand now. Yeah, fixing that does resolve the refleak. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

assert(!PyErr_Occurred()); \
PyObject *value = (value_expr); \
if (value == NULL) { \
goto error; \
Expand All @@ -7070,6 +7074,7 @@ _datetime_exec(PyObject *module)
goto error; \
} \
Py_DECREF(value); \
} \
} while(0)

/* timedelta values */
Expand Down Expand Up @@ -7117,6 +7122,8 @@ _datetime_exec(PyObject *module)
PyObject *max = create_timezone_from_delta(0, (23 * 60 + 59) * 60, 0, 0);
DATETIME_ADD_MACRO(d, "max", max);

#undef DATETIME_ADD_MACRO

/* Add module level attributes */
if (PyModule_AddIntMacro(module, MINYEAR) < 0) {
goto error;
Expand Down Expand Up @@ -7171,7 +7178,6 @@ _datetime_exec(PyObject *module)
Py_XDECREF(old_module);
return rc;
}
#undef DATETIME_ADD_MACRO

static PyModuleDef_Slot module_slots[] = {
{Py_mod_exec, _datetime_exec},
Expand Down
0