Menu
▾
▴
#2817 grid, pack, place on half-dead windows resurrects hash entry
I've explored this issue in depth for "grid" manager only, but others seems to be affected, too.
Code in tkGrid.c uses hash table to find stored information for managed windows. It's a hash table with one-word keys, mapping from tkwin to "struct Gridder*"; when some grid operation on a tkwin is requested for the first time, this structure is allocated and added to hash table. It is deleted from the hash table on DestoyNotify event.
A very bad thing happens when some <Destroy> binding executes a grid subcommand
on a destroyed window (it may be [grid info] or even [grid forget]). Binding is executed when DestroyNotify handler has already removed a Gridder* from the hash table; hence a Gridder for dead window is [re]created and added to the hash table. _No one will remove it from there any more_.
But it's not just a tiny memory leak: tkwin is going to become invalid as soon as Tk_DestroyWindow finishes, and its memory block can be reused for any other data structure. For some allocators (tclThreadAlloc at least) this structure will be probably another tkwin, for a new window. Any grid operation on this unhappy window will find an "existing" Gridder* and use it. Tk_CreateEventHandler is not called when Gridder* already exists. We now have a window that looks alright from the scripts' point of view, but (1) it doesn't rearrange slaves when resized, and (2) it won't be removed from the hash table, even if no grid calls in <Destroy> bindings occur this time. Both actions, (1) and (2), depend on an event handler (StructureNotifyMask -> GridStructureProc) that is not installed.
Destroyed tkwin addresses are reused surprisingly often. I was debugging the hv3 tabbed browser (where <Destroy> handler on notebook widget does [grid forget]), and after destroying a tab, the next new tab was disfunctional (the way described above) for about 10-30% of attempts. Thank God, such frequency enabled me to track down the problem.
It seems to be an optimal solution to conceal the window's death for "informational" and "removing/forgetting" geometry manager subcommands, returning an empty value with no error, and to generate an error if a script tries to add or configure a slave of the dead window, or a dead window itself.
Another variant, minimizing required effort and probably rather good semantically, is to add support for an _ultimate_ dead-window callback chain, called in Tk_DestroyWindow when no bindings or normal event handlers will be invoked any more. Removing tkwin from a hash table should happen in this callback.
A _pseudo-solution to be avoided_ is to reverse invocation order of [bind] handlers and Tk handlers for DestroyNotify event, ensuring that [bind] scripts are called first. Among other disadvantages, it is _unsafe_: adding a command trace for widget command deletion allows hooking into non-[bind] DestroyNotify handler chain.
On a second thought, another kind of solution seems worth to consider: adding some generic "window-specific data" API to be used e.g. by geometry managers, but potentially available to extensions as well. Doing hash lookup on tkwin pointers, instead of storing [a pointer to] data in tkwin itself, is silly.
Submitted patch #3021044, as an attempt to fix the problem for [grid] and propose a way for other managers as well.