#1537 Sometimes LineLayout::EndLineStyle is called when self->maxLineLength == 0

Bug
closed-works-for-me
scintilla (333)
5
2014-01-27
2013-10-08
No

I'm trying to track down the cause of a crash in Komodo when you repeatedly draw an indicator around the same words in the document. If I avoid calling ll->EndLineStyle from various points in Editor.cxx when ll->maxLineLength == 0 I don't crash immediately. If I leave the code as is, I can crash immediately.

The function is in the new "Multiple Selection: Add Next Occurrence of Current Word to Set"
which lets you change several instances of text in the document to a new value in parallel.
The default keybinding is "Ctrl-D", and if you keep pressing Ctrl-D when you hit the end
of the document, Komodo wraps to the beginning and re-applies the decoration to the
same pieces of text it saw before.

Unfortunately, that's not enough. Even with my "safeGetSpaceWidth", Komodo sometimes
will crash under Windows if I create these indicators quickly enough (and they're always
re-created at the same positions). I also needed to avoid redrawing existing identifiers,
and then I can hit Ctrl-D repeatedly, without crashing.

I can't say if the bug is completely in Scintilla. The "safeGetSpaceWidth" method, which
returns 0 if ll->maxLineLength == 0, avoids an obvious stacktrace, but I saw three other
stacktraces in the debugger on crashing, none of which involved scintilla. It could be that
a notification from drawing an indicator triggered other code, but it's hard to see
any kind of memory corruption (For the record, the three stacktraces involve
a notification handler written in JavaScript, unknown Python code, and C++ Mozilla code
involving a generic XUL routine).

In sum, avoiding calling LineLayout::EndLineStyle when self->maxLineLength == 0,
and avoiding redrawing existing indicators, prevents the crash.

And this only happens on Windows -- I can't reproduce on Linux or OSX. Which leads
me to believe there's a weird kind of memory corruption problem going on here.

Discussion

  • Neil Hodgson

    Neil Hodgson - 2013-10-08

    Could be a reentrance issue with a dead object -- that is an unexpected path of performing actions in a notification handler is destroying or reinitialising a layout object while it is still in use in calling code. Since numCharsBeforeEOL is checked in EndLineStyle, it may help problem isolation to add some assertions that numCharsBeforeEOL <= maxLineLength.

    You could try changing the line layout cache mode to perturb allocation behaviour and provoke failures. A change I've thought about in the past is to add a reference count to LineLayout to enable different parts of the code to own copies that will remain safe even if they are evicted from the cache.

    May also be worthwhile running with a memory checker like Valgrind, BoundsChecker or Clang's Address Sanitizer.

     
  • Eric Promislow

    Eric Promislow - 2013-10-09

    It looks like it is due to a notification handler firing.
    I thought we were running our handlers in a setTimeout to
    avoid re-entrancy, apparently not.

     
    • Neil Hodgson

      Neil Hodgson - 2013-10-09

      I'm interested in which notification or set of notifications are involved so some protection can be added to that situation. It will also help create test cases if the ref-count idea is implemented.

       
  • Neil Hodgson

    Neil Hodgson - 2014-01-27
    • labels: --> scintilla
    • status: open --> closed-works-for-me
    • assigned_to: Neil Hodgson
     
  • Neil Hodgson

    Neil Hodgson - 2014-01-27

    Need a reproducible case to advance this issue.

     

Log in to post a comment.