[Patches] [ python-Patches-1121234 ] Reference count bug fix

[SourceForge.net]

Patches item #1121234, was opened at 2005-02-12 07:02
Message generated for change (Comment added) made by loewis
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1121234&group_id=5470

Category: Tkinter
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Michiel de Hoon (mdehoon)
Assigned to: Martin v. Löwis (loewis)
Summary: Reference count bug fix

Initial Comment:
In the _loadtk method of the class Tk, there are two
calls self.tk.createcommand, one for 'tkerror' and one
for 'exit'. In the createcommand function in
_tkinter.c, there is a Py_XINCREF for the Tk widget and
one for the Python function (_tkerror or _exit). The
corresponding Py_XDECREFs are called (via
Tkapp_DeleteCommand) in PythonCmdDelete. The
Tkapp_DeleteCommand is called by the deletecommand in
the destroy method of class Misc in Tkinter.py.
However, the deletecommand is called only for commands
listed in self._tclCommands. Now, if a command is
created via the _register method in class Misc, the
command is appended to self._tclCommands. However, the
'tkerror' and 'exit' commands, created in the _loadtk
method of the class Tk, are not appended to
self._tclCommands. Hence, the '_tkerror' and '_exit'
functions as well as the Tk widget itself have an
incorrect reference count. The Tkapp_Dealloc routine in
_tkinter.c is not called and the Tk widget is not
destroyed.
The attached patch simply adds a
self._tclCommands.append for 'tkerror' and 'exit'. With
this patch, a Tk widget is destroyed and Tkapp_Dealloc
is called correctly.

----------------------------------------------------------------------

>Comment By: Martin v. Löwis (loewis)
Date: 2005-03-01 09:12

Message:
Logged In: YES 
user_id=21627

Thanks for the patch, applied as

Tkinter.py 1.182 and 1.181.2.1
NEWS 1.1248 and 1.1193.2.23

I added the test for _tclCommands is None, please verify the
code.

BTW, how would I make the exit command work? It seems that
it does not
properly cause a Python exit, but instead causes a TclError.

----------------------------------------------------------------------

Comment By: Michiel de Hoon (mdehoon)
Date: 2005-02-28 04:19

Message:
Logged In: YES 
user_id=488897

If we use _register without modification, we'll end up with
a different command name:
>>> from Tkinter import Label
>>> l = Label(text="some text")
+ Tkinter created command -1214096332_tkerror
+ Tkinter created command -1208510068_exit
+ Tkinter created command -1208511348destroy
>>>
whereas the command names should be "_tkerror", "_exit".
We can modify the _register functon so that it takes an
optional "name" keyword argument. However, it will mean that
we create the command through the CallWrapper instead of the
command directly. For "_exit", if I understand the code
correctly, it would mean that the raise in _exit is obscured
by the raise in CallWrapper, which doesn't seem the right thing.

So I would suggest to add a "if self._tclCommands==None:" to
the patch instead of going through _register. I'll update
the patch if you agree.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2005-02-27 20:27

Message:
Logged In: YES 
user_id=21627

Wouldn't it be better to use _register for _tkerror and
_exit as well? This patch overwrites self._tclCommands, but
the object may already have a _tclCommands attribute.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1121234&group_id=5470