Tk Source Code

View Ticket
Login
Ticket UUID: bdd1f64523f509f2632cdaaed4447de8c5abb7cb
Title: Assertion failed when switching -elide in the revised text widget
Type: Bug Version: revised_text
Submitter: anonymous Created on: 2025-01-28 17:31:06
Subsystem: 18. [text] Assigned To: fvogel
Priority: 5 Medium Severity: Critical
Status: Closed Last Modified: 2025-02-02 16:59:11
Resolution: Fixed Closed By: fvogel
    Closed on: 2025-02-02 16:59:11
Description:
Hello François Vogel, Gregor Cramer,

TIP 466: Revised Implementation of the Text Widget

Thanks for your wonderful work. I tried it and found bug/crash when using elide feature. Steps to reproduce bug:
 (1) Code from https://github.com/tcltk/tk/tree/revised_text/generic
 (2) Using Debug-Build, run following code
      pack [text .t]
      .t tag configure tag_ellipse  -elide 0
      .t tag configure tag_hidden   -elide 1
      .t insert end "text_ellipse" tag_ellipse
      .t insert end "text_hidden"  tag_hidden
      .t tag configure tag_ellipse  -elide 1
      .t tag configure tag_hidden   -elide 0
      --> assert failure in tkTextBTree.c TkBTreeFindStartOfElidedRange()
          assert(SegmentIsElided(sharedTextPtr, segPtr, textPtr));

If this assert is ignored then there is crash later on in my application. I would be happy to provide any more details on this, please just email me.

Many Thanks!
User Comments: fvogel added on 2025-02-02 16:59:11:
All right, thanks. Closing the ticket.

nalingupta added on 2025-02-01 18:46:46:

Thanks for your effort and fix. My apologies for incorrect linked list, I would be more careful next time.

Unfortunately there are multiple issues due to elide feature when using my application and we need to solve them one by one. I narrowed down the issue and then I created minimal code-snippet to raise ticket so that it is easy to debug.

My suggestion would be to adopt your fix and close this ticket.

I would raise another ticket for next issue I have encountered. Does this sound good to you?


fvogel added on 2025-02-01 15:52:50:

Non-regression test btree-14.3 added in [b8129965]. Crashes before the fix, passes with the fix.


fvogel added on 2025-02-01 15:37:46:

Confirmed. Trying fix b., i.e. calling:

    lastBranchPtr = TkBTreeFindStartOfElidedRange(sharedTextPtr, NULL, (*firstSegPtr)->prevPtr);
I immediately encounter a failed assertion in TkBTreeFindStartOfElidedRange() due to (*firstSegPtr)->prevPtr being NULL. This is easily reproduced by running the first lines of test text-11.9. It's gonna be tough to think about all possible cases.

If fix a. is OK for you I would go for it.


fvogel added on 2025-02-01 15:26:15:

Many thanks for your input. There are small errors in your post (notably segment order inversion in the linked list, the 3rd and 2nd segments are in the wrong order - the picture above that list is however correct), but it was definitely very useful to help me in the debug process.

I have understood what's happening. When toggling the -elide state of seg4 (the "text_hidden" segment), in the first iteration of the while (1) loop, the lastBranchPtr is obtained by calling TkBTreeFindStartOfElidedRange(). This function supposes (and checks through asserts) that the segment from which it is requested to start its search for the branch segment is elided. With the given reproducible script, at the time this gets called, that segment is not seen as elided since the elide state of the tag pertaining to this segment already has its state toggled to the desired new state (here: 0). And the assert (correctly) triggers.

I can think of at least two possible fixes:

a. Temporarily restore the elide state of the tag before calling TkBTreeFindStartOfElidedRange(), and do the reverse manoeuver after this call. This is done already in the code for finding the elide state of the segment predecessing the start of the considered range, that is to set the value of 'actualElided'.
b. Instead of starting the search for the branch segment from the first segment of the range where the elide state must be toggled, start the search from the segment that precedes it. This must work because this predecessing segment is always elided (actualElided=1) at this place of the code.

Fix a. is easy to understand, is straightforward to implement, and is safe. I have implemented it in [4bce1cc132].

Fix b. is more elegant (instead of passing *firstSegPtr, just pass (*firstSegPtr)->prevPtr to TkBTreeFindStartOfElidedRange()) but it is more difficult to do correctly since there might be cases where (*firstSegPtr)->prevPtr could be NULL, which would need to be handled by looking at the previous line (and what if there is no previous line, and so on). I have to think about this.

At this point, could you perhaps try fix a. and confirm whether it works in your large aplication? We could perhaps decide to be satisfied enough by this fix.


nalingupta added on 2025-01-31 01:44:09:
Point-1
=======
> If this assert is ignored then there is crash later on in my application.
>>I'm not reproducing this...ignored the given test case succeeds as expected.
This is correct. My remark was misleading. I have an application heavily using elide feature and it crashes after few more insert/toggle operations.

Point-2
=======
There are two possibilities:
 1. The assert is correct and there is a bug before causing it to fail
 2. The assert is not valid in all scenarios

Currently I am unsure which possibility is correct. I have done following analysis, but I need to consider more scenarios

Details of analysis so far (conclusion at end)
==========================

analysis-step-1: Snapshot before last statement
-----------------------------------------------
seg3 and seg4 are grouped together in same section which is elided=1 i.e. not visible in widget

 seg1--seg2--seg3--seg4--seg5--seg6--seg7--seg8
 mark  brch  char  char  link  mark  mark  char
  |     |     |     |     |     |     |     | 
   \    /      \   /       \    \     /    /
    \  /        \ /          \   \   /   /
  section1    section2          section3
   (len=2)     (len=2)           (len=4)

seg3:  "ellipse_text" "tag_ellipse"->elide=1
seg4:  "hidden_text"  "tag_hidden"->elide=1

-(struct TkTextMyBTree *) 0x000001b9f086e9f0
 -rootPtr 0x000001b9f08690e0 Node *
  ...
  -linePtr 0x000001b9f0858fa0 TkTextLine *
   ...
   -segPtr 0x000001b9f08787a0 TkTextSegment *
    +typePtr 0x00007ff8d7871c30 tkTextLeftMarkType "mark"
    -nextPtr 0x000001b9f0872900 TkTextSegment *
     +typePtr 0x00007ff8d7869e40  tkTextCharType "character"
     -nextPtr 0x000001b9f0862c40 TkTextSegment *
      +typePtr 0x00007ff8d7869e78  tkTextBranchType "branch"
      -nextPtr 0x000001b9f0862bc0 TkTextSegment *
       +typePtr 0x00007ff8d7869e40  tkTextCharType "character"
       -nextPtr 0x000001b9f0875410 TkTextSegment *
        +typePtr 0x00007ff8d7869eb0  tkTextLinkType "connection"
        -nextPtr 0x000001b9f085d530 TkTextSegment *
         +typePtr 0x00007ff8d7871c68  tkTextRightMarkType "mark"
         -nextPtr 0x000001b9f085aca0 TkTextSegment *
          +typePtr 0x00007ff8d7871c68  tkTextRightMarkType "mark"
          -nextPtr 0x000001b9f0858f20 TkTextSegment *
           +typePtr 0x00007ff8d7869e40  tkTextCharType "character"
           +nextPtr 0x0000000000000000 TkTextSegment *

analysis-step-2
---------------
Now when last statement to toggle "tag_hidden"->elide=0 is being processed, the call stack is as follows:

TkConfigureTag() Line 1274
 TkBTreeUpdateElideInfo() Line 9013
  UpdateElideInfo () Line 8862
  {
   ...
   if (tagPtr && reason == ELISION_HAS_BEEN_CHANGED) {
     tagPtr->elide = 1 // line 8676: temp setting elide's starting state
   }
   ...
   actualElided = 1 // line 8691: because seg4 "hidden_text" current state is elide=1
   ...
   tagPtr->elide = 0 // line 8711: restoring elide's final state
   ...
   int shouldBeElided =0 // line 8789: because seg4 "hidden_text" final state elide=0
   ...
   TkBTreeFindStartOfElidedRange( seg4 is passed here )
   {
    ...
    assert(SegmentIsElided(//seg4 has tagPtr->elide=0 => hence assert fails
              //but actually seg3+seg4 are elided
              //Possibility-1: tagPtr->elide should have been "1" before calling this function
              //Possibility-2: this assert statement is not valid for this particular call-stack / scenario
    // This function returns seg2 "branch"

fvogel added on 2025-01-29 20:55:37:

> If this assert is ignored then there is crash later on in my application.

I'm not reproducing this. I don't know about your application, but when the assert in tkTextBTree.c:14855 is ignored, then the assert in tkTextBTree.c:14777 triggers, and when the latter is also ignored the given test case succeeds as expected.

That said, the assert on line 14855 makes sense. It reads:

    assert(SegmentIsElided(sharedTextPtr, segPtr, textPtr));
which in a function named TkBTreeFindStartOfElidedRange makes quite some sense.


fvogel added on 2025-01-28 20:53:11:

Many thanks for the report, with special thanks for the short script allowing to reporduce, that's very important and helpful.

We are chasing a bug with text elision (like to one you are reporting) for quite a long time, for instance it happens when using tablelist (see [73d8d94916]). This may or may not be due to the same root issue. The call stack here is:

>	tcl9tk90.dll!TkBTreeFindStartOfElidedRange(const TkSharedText * sharedTextPtr, const TkText * textPtr, const TkTextSegment * segPtr) Ligne 14855	C
 	tcl9tk90.dll!UpdateElideInfo(TkSharedText * sharedTextPtr, TkTextTag * tagPtr, TkTextSegment * * firstSegPtr, TkTextSegment * * lastSegPtr, unsigned int reason) Ligne 8862	C
 	tcl9tk90.dll!TkBTreeUpdateElideInfo(TkText * textPtr, TkTextTag * tagPtr) Ligne 9013	C
 	tcl9tk90.dll!TkConfigureTag(Tcl_Interp * interp, TkText * textPtr, const char * tagName, int redraw, int objc, Tcl_Obj * const * objv) Ligne 1271	C
 	tcl9tk90.dll!TkTextTagCmd(TkText * textPtr, Tcl_Interp * interp, __int64 objc, Tcl_Obj * const * objv) Ligne 453	C
 	tcl9tk90.dll!TextWidgetObjCmd(void * clientData, Tcl_Interp * interp, int objc, Tcl_Obj * const * objv) Ligne 2600	C
 	tcl90.dll!Dispatch(void * * data, Tcl_Interp * interp, int dummy4602) Ligne 4641	C
 	tcl90.dll!TclNRRunCallbacks(Tcl_Interp * interp, int result, NRE_callback * rootPtr) Ligne 4656	C
 	tcl90.dll!TclEvalObjEx(Tcl_Interp * interp, Tcl_Obj * objPtr, int flags, const CmdFrame * invoker, int word) Ligne 6116	C
 	tcl90.dll!Tcl_EvalObjEx(Tcl_Interp * interp, Tcl_Obj * objPtr, int flags) Ligne 6097	C
 	tcl90.dll!Tcl_RecordAndEvalObj(Tcl_Interp * interp, Tcl_Obj * cmdPtr, int flags) Ligne 182	C
 	tcl90.dll!StdinProc(void * clientData, int dummy739) Ligne 793	C
 	tcl90.dll!Tcl_NotifyChannel(Tcl_Channel_ * channel, int mask) Ligne 8628	C
 	tcl90.dll!ConsoleEventProc(Tcl_Event * evPtr, int flags) Ligne 1446	C
 	tcl90.dll!Tcl_ServiceEvent(int flags) Ligne 725	C
 	tcl90.dll!Tcl_DoOneEvent(int flags) Ligne 1024	C
 	tcl9tk90.dll!Tk_MainLoop() Ligne 2093	C
 	tcl90.dll!Tcl_MainExW(__int64 argc, wchar_t * * argv, int(*)(Tcl_Interp *) appInitProc, Tcl_Interp * interp) Ligne 563	C
 	tclsh90.exe!wmain(int argc, wchar_t * * argv) Ligne 144	C

It is not the same as in [73d8d94916] but the suspicious code area is the same. The fact we have a small script demonstrating the bug should help in finding the cause. Thanks again!