This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Title: tkinter.Image.__del__ can throw an exception if module globals are destroyed in the wrong order
Type: behavior Stage: patch review
Components: Tkinter Versions: Python 3.4, Python 3.5
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: JanKanis, pitrou, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2013-06-05 12:09 by JanKanis, last changed 2022-04-11 14:57 by admin. This issue is now closed.

File name Uploaded Description Edit
3fdd418be9f8.diff JanKanis, 2013-06-05 12:21 review
Repositories containing patches
Messages (13)
msg190654 - (view) Author: Jan Kanis (JanKanis) Date: 2013-06-05 12:09
While fixing #18132 I noticed that when closing the turtledemo window an exception is thrown (and ignored) because the __del__ tries to catch a TclError exception, but the TclError global has already been deleted and set to None. 

I could only reproduce this in my self built pydebug build, at least on revision dfcb64f51f7b, but not on the v3.3.2 tag. (compiled with --with-pydebug on linux x86-64)


me@mymachine ~/d/cpython> ./python -m turtledemo
Exception ignored in: <bound method PhotoImage.__del__ of <tkinter.PhotoImage object at 0x7fbddb4f0a18>>
Traceback (most recent call last):
  File "~/devel/cpython/Lib/tkinter/", line 3330, in __del__
TypeError: catching classes that do not inherit from BaseException is not allowed
msg190655 - (view) Author: Jan Kanis (JanKanis) Date: 2013-06-05 12:21
added a fix
msg190656 - (view) Author: Jan Kanis (JanKanis) Date: 2013-06-05 12:24
Oops, the diff includes patches for both this issue and #18132, that should be just commit 3fdd418be9f8.
msg219294 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-05-28 18:44
My worry about the patch is that is is a bandage covering up a problem elsewhere.

1. You indicate that you only have a problem with your custom build, but not on a standard build. This seems odd. Perhaps the problem is with your build.

2. turtledemo.__main__ only calls root.destroy indirectly via
        root.wm_protocol("WM_DELETE_WINDOW", self._destroy)
    def _destroy(self):
Let's assume that .destroy is called when the exception happens (verifiable by adding "print('destroy called')".)

3. The purpose of Tk.destroy is to cleanly deconstruct the gui before python's random cleanup. Should it have cleared PhotoImage?

Image and Variable (and subclasses) are the only classes in tkinter.__init__ with __del__ methods. Both have a, so Variable potentially has the same problem as Image. But neither seem to have a problem in routine tkinter use.

4. Why is there a PhotoImage to be deleted? Idle Find in Files does not find 'PhotoImage' in turtledemo/*.py. I believe it is created as follows. turtledemo.DemoWindonw.__init__ contains this odd pair of lines that initializes a turtle.Screen instance with turtle.TurtleScreen.__init__ (TurtleScreen subclasses TurtleScreenBase, not Screen).
        self.screen = _s_ = turtle.Screen()
        turtle.TurtleScreen.__init__(_s_, _s_._canvas)
The latter call creates a collection of default turtles (_shapes) that includes 'blank', a 1x1 (pixel) blank PhotoImage that gets tkinter._default_root as (default) master.

I suspect that adding either of these lines to _destroy would prevent the TclError.
    del self.screen  # before self.root.destroy()
    import tkinter; tkinter._default_root.destroy()  # after self.root

5. While it seems like a buglet for turtle to create a hidden PhotoImage, with an anonymous master, and not clean it up somehow; and while I am puzzled if this never happens with standard builds; it seems plausible that we could patch Variable/Image.__delete__ to be sure they do not leak an exception.

Serhiy, what do you think?
msg219298 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-05-28 20:48
1. 3.3 is in security fixes only mode and shutdowning mechanism was changed in 

2. Looks as some Tk root was not explicitly destroyed and deleted during 
shutdown stage.

3. Tk.destroy does nothing with images, it destroys only subcomponents and 
commands. In any case it destroys only Tcl/Tk objects, not Python objects 
which wrap them.

4. I think we shouldn't use _default_root outside the tkinter package. This is 
implementation detail.

5. Agree, we could patch Variable/Image.__del__, and proposed patch looks 
correct (other way is pass TclError as default value for optional parameter: 
"def __del__(self, TclError=TclError)", both ways are widely used). But I 
prefer first try to reproduce this in tests.
msg219488 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-01 05:54
I re-read turtledemo and it seems that is could exit without calling root.destroy. The 'if main' block follows:

    demo = DemoWindow()
    RUN = True
    while RUN:
            #print("ENTERING mainloop")
        except AttributeError: <swallow and continue, for debug?>
        except TypeError: <swallow and continue, for debug?>
            RUN = False

If a non-Attribute/Type/Error occurs, it is swallowed and the process shuts down without root.destroy, which depends on closing the root window. It seems to me that for production use, everything after the first line should be replaced by demo.root.mainloop(), as is standard for tkinter apps, so that if *any* exception occurs, the process stops and a proper traceback gets printed (before shutdown).
msg219718 - (view) Author: Jan Kanis (JanKanis) Date: 2014-06-03 19:51
The main block has been like that from the beginning of recorded time. I could see a use for this if the turtle demo allowed changing of the code in the gui, but it doesn't.
msg219720 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-03 20:03
I am planning the make the simplifying change. But does is solve the problem you had?
msg219941 - (view) Author: Jan Kanis (JanKanis) Date: 2014-06-07 14:29
I tried changing the last block in turtulemodule/ to

if __name__ == '__main__':
    demo = DemoWindow()
    print("ENTERING mainloop")

but that does not solve the problem:

> python3 -m turtledemo
ENTERING mainloop
Exception ignored in: <bound method PhotoImage.__del__ of <tkinter.PhotoImage object at 0x7ffef193b948>>
Traceback (most recent call last):
  File "/home/jan/test/lib/python3.4/tkinter/", line 3330, in __del__
TypeError: catching classes that do not inherit from BaseException is not allowed

so still the same error. (Although it is probably still good to make that change.)

Tested on cpython revision dfcb64f51f7b, so the same as I originally made the bugreport from.
msg219958 - (view) Author: Jan Kanis (JanKanis) Date: 2014-06-07 18:45
I have verified that DemoWindow._destroy(self) indeed gets called before the exception is raised. 

I did a bisect, on the default branch the bug was introduced by commit f0833e6ff2d2: "Issue #1545463: Global variables caught in reference cycles are now garbage-collected at shutdown."

I will try bisecting the 3.3 branch to se where the bug stops appearing.
msg219993 - (view) Author: Jan Kanis (JanKanis) Date: 2014-06-07 22:46
The 3.3 branch is not affected as the f0833e6ff2d2 changeset was never merged into that branch.

In the default branch the exception stops appearing after commit 79e2f5bbc30c: "Issue #18214: Improve finalization of Python modules to avoid setting their globals to None, in most cases." This changeset is also part of the current 3.4 tip (19172062e5c0), so 3.4 tip does not display the buggy behaviour. Judging by the commit message, that commit also supersedes the attached patch as it at least prevents the visible symptoms. I am not sure if there still might be a deeper issue that is hidden by the changeset as Terry suggested.
msg220010 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-08 01:35
Thank you for the investigation. Unless there is a problem in 2.7, which you have not mentioned, this should be closed as out-of-date, or maybe fixed.
msg220032 - (view) Author: Jan Kanis (JanKanis) Date: 2014-06-08 12:42
I tested 2.7 tip (6dfbe504f659), which does not show the problem, as expected. 

I think there was a real bug in that the tkinter.TclError global was being set to None on exit, but a TclError being raised is expected if I go by the comment in tkinter. The bug was fixed in commit 79e2f5bbc30c.
Date User Action Args
2022-04-11 14:57:46adminsetgithub: 62341
2014-06-08 12:42:34JanKanissetstatus: open -> closed
resolution: out of date
messages: + msg220032

versions: - Python 2.7
2014-06-08 01:35:06terry.reedysetmessages: + msg220010
2014-06-07 22:46:34JanKanissetmessages: + msg219993
2014-06-07 18:45:41JanKanissetmessages: + msg219958
2014-06-07 14:29:54JanKanissetmessages: + msg219941
2014-06-03 20:03:51terry.reedysetmessages: + msg219720
2014-06-03 19:51:03JanKanissetmessages: + msg219718
2014-06-01 05:54:52terry.reedysetmessages: + msg219488
2014-05-28 22:54:28terry.reedysetversions: + Python 3.5, - Python 3.3
2014-05-28 20:48:56serhiy.storchakasetmessages: + msg219298
2014-05-28 18:44:05terry.reedysetnosy: - gpolo
messages: + msg219294
2013-09-15 11:22:29serhiy.storchakasetnosy: + terry.reedy, gpolo, serhiy.storchaka
stage: patch review
type: behavior

versions: + Python 2.7, Python 3.3
2013-06-05 13:24:56r.david.murraysetnosy: + pitrou
2013-06-05 12:24:56JanKanissetmessages: + msg190656
2013-06-05 12:21:26JanKanissetfiles: + 3fdd418be9f8.diff
keywords: + patch
2013-06-05 12:21:01JanKanissethgrepos: + hgrepo196
messages: + msg190655
2013-06-05 12:09:36JanKaniscreate