Menu

#2808 Empty binding scripts

obsolete: 8.5.8
closed-fixed
69. Events (88)
5
2010-05-31
2010-05-25
Steven
No

This may be a very minor windows only bug, but activestate tcl-8.5.8.2 (WinXP) crashes immediately when i assign a null binding to button-1 on a treeview widget, and click on the widget.
The code that causes it is
$w.tree tag bind click1 <Button-1> {}
When using click1 as a tag such as
$w.tree insert {} 0 -values $values -tag [list click1 click2 treefont]

Problem is easily solved by removing null binding.

Sorry for such a shitty bug report, but it may be interesting to someone

Project is scidvspc , subversion release 42
http://scidvspc.svn.sourceforge.net/viewvc/scidvspc?view=rev&revision=42

Discussion

  • Joe English

    Joe English - 2010-05-25

    I think I've seen this before. I thought it was fixed. Will check.

     
  • Joe English

    Joe English - 2010-05-25

    Nope, I haven't seen this before, and it hasn't been fixed.

    The treeview widget is apparently not using Tk_BindEvent() et. al., in the way in which they were designed to be used. More than that I cannot say at this time; will look into it later.

    Thanks for the bug report, it's actually quite useful.

     
  • Joe English

    Joe English - 2010-05-25
    • priority: 5 --> 7
     
  • Steven

    Steven - 2010-05-25

    I had a go at reproducing it with the 8.5 tk demo
    " 4. A multi-column list of countries"
    and it is easily reproducible here.
    To make the demo crash make these changes.

    + $w.tree tag bind click1 <Button-1> {}
    foreach {country capital currency} $data {
    - $w.tree insert {} end -values [list $country $capital $currency]
    + $w.tree insert {} end -values [list $country $capital $currency] -tag click1
    and click on treeview.

    Using
    + $w.tree tag bind click1 <Button-1> {puts ok}
    is fine.
    -----------------------------------------
    [
    Can anyone tell me how to figure out how many ttk_treeview rows are in a widget after it has been resized. I think i saw that "treeview configure -rowheight" is in cvs... but is there another way
    I currently use:

    set fontspace [font metrics [ttk::style lookup [$w cget -style] -font] -linespace]
    set fontspace [expr $fontspace*120/72] ; # hack to make it work better
    set num_rows [expr {int( [winfo height $w] / $fontspace)}]
    ]

     
  • Joe English

    Joe English - 2010-05-27

    Apparently Tk_BindEvent() does not like it if an empty script is passed to Tk_CreateBinding(). This is the apparent cause of the coredump.

    Other widgets (text, canvas) interpret [$w tag foo <...> {}] to mean "remove this tag binding entirely"; ttk::treeview widget updated to do the same.

    Additionally: the [text] and [canvas] widgets can be made to crash by appending an empty script to a binding:

    pack [text .t]
    .t tag bind empty <ButtonPress-1> +
    .t insert end "CLICK ME" empty

     
  • Joe English

    Joe English - 2010-05-27
    • summary: Uncommon win32 ttk::treeview crash --> Empty tag bindings crash ttk::treeview
     
  • Joe English

    Joe English - 2010-05-27

    Can also crash the canvas this way:

    pack [canvas .c]
    .c bind empty <ButtonPress-1> +
    .c create rectangle 0 0 100 100 -tags empty
    event generate .c <ButtonPress-1> -x 100 -y 100

    ... although programmatically using [event generate] call doesn't reliably trigger the bug; clicking manually seems to always do the trick. Same goes for text widget script above.

     
  • Joe English

    Joe English - 2010-05-27
    • priority: 7 --> 3
     
  • Donal K. Fellows

    I'd like to squelch that problem in Tk_CreateBinding rather than everywhere else.

     
  • Donal K. Fellows

    • assigned_to: jenglish --> dkf
    • labels: 318662 --> 69. Events
    • priority: 3 --> 9
     
  • Steven

    Steven - 2010-05-27

    Is this relevant ?
    On my linux system (fedora7, wish 8.5.7) i can generate a crash
    by opening an interactive wish shell, pasting your canvas script,
    and then pressing CR another couple of times
    Wish 8.4 is the same.

    /root#wish8.5
    % pack [canvas .c]
    % .c bind empty <ButtonPress-1> +
    % .c create rectangle 0 0 100 100 -tags empty
    % event generate .c <ButtonPress-1> -x 100 -y 100

    *** glibc detected *** wish8.5: malloc(): memory corruption (fast): 0x0831e907 ***

     
  • Joe English

    Joe English - 2010-05-28

    OK, here's what's going on:

    A PatSeq in a Tk_BindingTable can map to one of two things:
    either a Tcl script (the normal case) or a TkBindEvalProc.
    Scripts are added with Tk_CreateBinding(), and TkBindEvalProcs
    are added by the internal routine TkCreateBindingProcedure().

    [ Archaeology: the latter was added sometime between Tk 4.2
    and Tk 8.0. It's called from win/tkWinSclrbr.c, carbon/tkMacOSXScrlbr.c
    (formerly mac/tkMacScrlbr.c), and used to be called from
    win/tkWinMenu.c. The TkCreateBindingProcedure() calls
    for Windows menus have since been replaced with ordinary
    script bindings that call ::tk::WinMenuKey; the only remaining
    use of procedure bindings is for Windows and Carbon native scrollbars,
    where they are apparently used to compel Tk_BindEvent to enter a modal loop
    (This is a whole brand new level of WTF, but does not appear to
    be relevant to the bug at hand.) ]

    Moving on: the initial part of Tk_BindEvent() queries the binding
    table for matching pattern sequences, %-substitutes the corresponding
    scripts, and appends them to a Tcl_DString, with an extra '\0'
    separator between scripts. If the pattern maps to a binding procedure
    instead of a binding script, it records that in the 'pendingPtr' array
    and -- this is important -- also appends an extra '\0' to
    the Tcl_DString.

    The second part of Tk_BindEvent loops through the scripts in
    the Tcl_DString and evaluates them. In the case of an empty
    script, it executes the next binding procedure in the pendingPtr
    list.

    So finally: [$t tag bind foo <...> {+}] adds an empty binding script
    to the tag binding table. When the binding fires, the second
    part of Tk_BindEvent() interprets this to mean that the first part
    had recorded a binding procedure, not a binding script. The first
    part, of course, had done no such thing, and chaos ensues.

    I can think of a number of ways to patch this bug, but none
    at the moment that won't further increase the WTFs/min metric.

     
  • Joe English

    Joe English - 2010-05-28
    • summary: Empty tag bindings crash ttk::treeview --> Empty binding scripts
     
  • Joe English

    Joe English - 2010-05-30
    • assigned_to: dkf --> jenglish
    • priority: 9 --> 5
     
  • Joe English

    Joe English - 2010-05-31

    patch to generic/tkBind.c r1.65

     
  • Joe English

    Joe English - 2010-05-31

    Problem affects bindings in general:

    bind all <ButtonPress> {+}

    Then just start clicking on stuff; eventually it'll crash.

     
  • Joe English

    Joe English - 2010-05-31

    Attached patch (committed to HEAD and backported to core-8-5 branch) is a minimal change to fix the problem: Tk_CreateBinding() now silently ignore empty scripts.

    Error detection could be better -- [bind $foo <BadSequence> {+}] won't raise an error -- but since the 2-argument form [bind $foo <BadSequence>] doesn't currently raise an error either (apparently on purpose) I don't think that's a big deal.

    Not bothering to backport to 8-4-branch; this bug has been present since Tk 8.0 and nobody's noticed.

    A more aggressive patch that addresses the underlying problem is in preparation.

     
  • Joe English

    Joe English - 2010-05-31
    • status: open --> closed-fixed
     
  • Joe English

    Joe English - 2010-06-19

    Patch#3009998 committed; this fixes the underlying problem.