Tk Source Code

View Ticket
Login
Ticket UUID: 2f78c7c5ea70ee2fb363371beffc8ab47dcd0dc8
Title: text segfault with tablelist in TkBTreeLinesTo
Type: Bug Version: 8.6.4
Submitter: aspect Created on: 2015-12-17 05:09:09
Subsystem: 18. [text] Assigned To: fvogel
Priority: 8 Severity: Minor
Status: Closed Last Modified: 2016-02-09 00:20:07
Resolution: Fixed Closed By: fvogel
    Closed on: 2016-02-09 00:20:07
Description:
This is an odd one, triggered with tablelist on 8.6.4, 8.5.18 and fossil trunk.  All on linux-amd64; I haven't tested other platforms.

Two attachments:

 - script to trigger the SEGV

 - gdb transcript on current fossil trunk

Small variations of the script do *not* trigger the bug:  N=7 (or higher) and activating row 4, plus the final sequence {update; insert; delete} all seem to be required.
User Comments: fvogel added on 2016-02-09 00:20:07:

Better fix found when hunting down for [06c1433906].


fvogel added on 2015-12-26 21:25:18:
Fix merged into core-8-5-branch and trunk. Many thanks to Csaba Nemethi for his support and help!

fvogel added on 2015-12-23 16:38:09:
New test textDisp-9.14 added to check against regression regarding this bug.

fvogel added on 2015-12-23 16:22:53:
Again the script (without eaten-out line breaks...):

    package require Tk

    pack [text .t]
    for {set i 1} {$i < 6} {incr i} {
        .t insert end \nfoo$i
    }
    .t tag configure mytag1 -relief raised
    .t tag configure mytag2 -relief solid

    proc doit {} {
        .t tag add mytag1 4.0 5.0
        .t tag add mytag2 4.0 5.0
        after idle {
            .t tag remove mytag1 1.0 end
            .t tag remove mytag2 1.0 end
        }
        .t delete 1.0 2.0
    }

    doit  ; # must not crash

fvogel added on 2015-12-23 16:07:21:

Got it! Here is a pure Tk script triggering the crash:

package require Tk destroy .t
pack [text .t] for {set i 1} {$i < 6} {incr i} { .t insert end \nfoo$i } .t tag configure mytag1 -relief raised .t tag configure mytag2 -relief solid
proc doit {} { .t tag add mytag1 4.0 5.0 .t tag add mytag2 4.0 5.0 after idle { .t tag remove mytag1 1.0 end .t tag remove mytag2 1.0 end } .t delete 1.0 2.0 }
doit ; # must not crash

This script is seemingly minimal.

Understanding what Tablelist is doing (displayItems and activeTrace sometimes called at idle time, sometimes immediately forced) and how the issue got triggered in the text widget was not so easy.

With the fix [311ef109] I have proposed in branch bug-2f78c7c5ea this script does no longer crash.

The only remaining thing to do is to turn this into a new test of the Tk test suite, which is easy. Stay tuned!


fvogel added on 2015-12-23 11:21:18:
Correction: it should be possible to create a script triggering the crash without elided lines. I'm now seeing what's happening. The delete operation performs correctly, then the trace triggers, which runs 'tag remove' on the whole text. At this point, in TextRedrawTag, dInfoPtr is used to round down the starting index for tag redrawing (since tags are only drawn when they are displayed on screen), and this dInfoPtr has some garbage in it because the first display line was freed by the delete operation and the new dInfo were not yet re-computed.

The fix is therefore to call UpdateDisplayInfo if (dInfoPtr->flags & DINFO_OUT_OF_DATE.

Now I'm concentrating on building a small Tk-only test case that I could then turn into a non regression test.

fvogel added on 2015-12-22 21:41:51:

fossil bisect points to this commit as the first bad one:

[04d70b6f] Fixed TextChanged caller of FindDLine for correct taking into account of elided newlines (user: fvogel, tags: bug-7703f947aa)

This is consistent with your report (OK in 8.6.3, KO in 8.6.4, KO in text-elided and KO tip-438 branch).

It's not immediately obvious to me what is wrong in the above commit. I must dive into this carefully and need some quiet time to do so. It is part of a large work that used to live in branch bug-7703f947aa (fix for bug [7703f947aa]). I have forgotten the details for a long time, of course, but all this was quite complicated.

Do you use elided lines in Tablelist? I think a pure-Tk test case demonstrating the crash must use elided lines. A pure-Tk script showing the issue would help me a real lot.


nemethi added on 2015-12-21 19:57:33:
Sorry, I meant Tk 8.6.3 and 8.6.4 rather than 6.3 and 6.4. :-)

nemethi added on 2015-12-21 19:25:48:
That write trace callback is triggered at several places in the Tablelist code.  In this example, it will also be triggered by ".t insert end "foo$i"" and ".t delete 0 0".  The crash happens when the trace callback is triggered by the delete subcommand.

To see this, I have inserted the following lines at the beginning of the trace callback tablelist::moveActiveTag:

set callerProc [lindex [info level -1] 0]
puts "moveActiveTag called by $callerProc"
catch {
    set caller2Proc [lindex [info level -2] 0]
    puts "  $callerProc called by $caller2Proc"
}

I have tried to reproduce the text widget crash with the aid of a script that doesn't use Tablelist, but, unfortunately,  w/o success.  Here is that test script:

set w .t
text $w
pack $w

$w tag configure curRow -borderwidth 1 -relief raised
$w tag configure active -borderwidth 1 -relief solid

set N 6 ;# or higher

$w insert end foo0
for {set i 1} {$i < $N} {incr i} {
    $w insert end \nfoo$i
}

focus $w
update

set activeLine 4
$w tag remove curRow 1.0 end
$w tag remove active 1.0 end
$w tag add active $activeLine.0 $activeLine.end

$w insert end \nfoo$i

set activeLine 4
$w tag remove curRow 1.0 end
$w tag remove active 1.0 end
$w tag add active $activeLine.0 $activeLine.end

$w delete 1.0 2.0

set activeLine 3
$w tag remove curRow 1.0 end
$w tag remove active 1.0 end
$w tag add active $activeLine.0 $activeLine.end

One more remark: I couldn't reproduce the crash in the original script using Tcl/Tk 6.3.  I don't have version 6.4, but I once built Tk using the "text-elided" branch, and recently using the "tip-438" branch.  The original script crashes with both Tk versions.  From this I conclude that the crash is probably caused by some change in the text widget code made between Tk 6.3 and 6.4.

fvogel added on 2015-12-20 11:47:40:
Further trimming down:

  package require tablelist
  pack [tablelist::tablelist .t -columns {0 text}]
  set N 700
  for {set i 0} {$i < $N} {incr i} {
      .t insert end "foo$i"
  }

  .t activate 3
  focus .t
  .t insert end "foo$i"
  .t delete 0 0

Now paste the first part (before the blank line). So far so good. Preparation of the widget contents.
Then paste at once the second group of commands (the last four lines): crash!

From what I saw in the tablelist code, activation of row 3 triggers a write trace on variable "activeRow" which runs a proc which first command is '$w tag remove curRow 1.0 end' and that last one generates the text widget crash.

fvogel added on 2015-12-20 11:25:55:
Reproduced on Win 7 and Vista with core-8-5-branch and trunk (more difficult to get with trunk though).

A few observations:

- Does not crash with 8.5.15 nor 8.6.16, nor 8.4.20.

- It does not crash when script is pasted in wish, only when pasted in tclsh.

- It does not crash, even in tclsh, when lines are pasted one by one, but when the full script is pasted at once. If the script is pasted at once up to but not including the final delete, and then the delete is pasted, then it does not crash either.

- It does not crash if "update idletasks" is inserted between the final insert and delete commands.

- It is much more difficult to crash if anything (including a comment) is inserted between the final insert and delete commands.

- So far I couldn't manage to kick tablelist out of the problem.


Reproducible with the following:

  package require tablelist
  namespace import tablelist::tablelist

  tablelist .t -columns {0 text}
  pack .t

  set N 700

  for {set i 0} {$i < $N} {incr i} {
      .t insert end "foo$i"
  }

  .t activate 3
  focus .t
  update
  .t insert end "foo$i"
  .t delete 0 0

fvogel added on 2015-12-17 06:50:30:
Since tablelist is a pure Tcl package this must be a text widget crash.

What would really help is a pure Tk script. I'm pretty sure there should be a way to trigger this without tablelist.

Can anyone exhibit such pure Tk script, perhaps by trimming down what tablelist does? Csaba, if you read this...?

aspect added on 2015-12-17 05:29:14:
A little more info:  the gdb trace identifies "tablelist::activeTrace .t data activeRow w" as the last Tcl command.  Digging into tablelist, the actual call that fails is:

  .t.body tag remove curRow 1.0 end

on line 3258 of tablelist5.14/scripts/tablelistUtil.tcl, at the start of proc tablelist::moveActiveTag

Attachments: