Skip to content

gh-123504: Fix reference leak in initialization of _tkinter #123505

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
vstinner merged 9 commits into from
Sep 3, 2024
Merged

gh-123504: Fix reference leak in initialization of _tkinter #123505

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cddd021
Fix reference leak in initialization of _tkinter
ZeroIntensity Aug 30, 2024
a9eb482
Add NEWS entry.
ZeroIntensity Aug 30, 2024
36c65af
Switch to `PyModule_AddObjectRef`
ZeroIntensity Aug 30, 2024
cc291e4
Switch to a module clear function.
ZeroIntensity Aug 30, 2024
df9265f
Add back missing newline.
ZeroIntensity Aug 30, 2024
4f1f4e4
Fix formatting.
ZeroIntensity Aug 30, 2024
a77abba
Switch to `Py_CLEAR`
ZeroIntensity Aug 30, 2024
f35f2a0
Update Misc/NEWS.d/next/Library/2024-08-30-09-01-35.gh-issue-123504.l…
ZeroIntensity Sep 3, 2024
fc34562
Use dot notation for module initializer.
ZeroIntensity Sep 3, 2024
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2024-08-30-09-01-35.gh-issue-123504.lJ9_BB.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed reference leak in the initalization of :mod:`tkinter`.
12 changes: 8 additions & 4 deletions Modules/_tkinter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3419,8 +3419,9 @@ PyInit__tkinter(void)
#endif

Tkinter_TclError = PyErr_NewException("_tkinter.TclError", NULL, NULL);
if (PyModule_AddObjectRef(m, "TclError", Tkinter_TclError)) {
if (PyModule_AddObject(m, "TclError", Tkinter_TclError)) {
Copy link
Member

@serhiy-storchaka serhiy-storchaka Aug 30, 2024

Choose a reason for hiding this comment

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

Tkinter_TclError should be a strong reference to _tkinter.TclError. Even if you remove "TclError" from the the module dict, the use of Tkinter_TclError should not crash.

BTW, do not use PyModule_AddObject(), it is broken beyond repair. Use PyModule_AddObjectRef() or PyModule_Add(), what is more convenient.
Copy link
Member Author

@ZeroIntensity ZeroIntensity Aug 30, 2024

Choose a reason for hiding this comment

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

Ok, where should the strong reference to Tkinter_TclError be decref'd? Should we add a module clear function?
Py_DECREF(m);
Py_DECREF(Tkinter_TclError);
return NULL;
}

Expand Down Expand Up @@ -3470,20 +3471,23 @@ PyInit__tkinter(void)
}

Tkapp_Type = PyType_FromSpec(&Tkapp_Type_spec);
if (PyModule_AddObjectRef(m, "TkappType", Tkapp_Type)) {
if (PyModule_AddObject(m, "TkappType", Tkapp_Type)) {
Py_DECREF(m);
Py_DECREF(Tkapp_Type);
return NULL;
}

Tktt_Type = PyType_FromSpec(&Tktt_Type_spec);
if (PyModule_AddObjectRef(m, "TkttType", Tktt_Type)) {
if (PyModule_AddObject(m, "TkttType", Tktt_Type)) {
Py_DECREF(m);
Py_DECREF(Tktt_Type);
return NULL;
}

PyTclObject_Type = PyType_FromSpec(&PyTclObject_Type_spec);
if (PyModule_AddObjectRef(m, "Tcl_Obj", PyTclObject_Type)) {
if (PyModule_AddObject(m, "Tcl_Obj", PyTclObject_Type)) {
Py_DECREF(m);
Py_DECREF(PyTclObject_Type);
return NULL;
}

Expand Down