[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwrite-devel
Subject:    deletion of TextRange
From:       Dominik Haumann <dhaumann () kde ! org>
Date:       2013-03-16 17:38:29
Message-ID: 1399310.LOBd6YD02K () eriador
[Download RAW message or body]

Hi Christoph,

in short: to me it looks like there is a dangling pointer in the cached
range list in the text blocks. See the following explanation:

with respect to bug: https://bugs.kde.org/show_bug.cgi?id=313759 and its valgrind \
trace: ==10276== Invalid read of size 4
==10276==    at 0x62F2116: Kate::TextCursor::lineInternal() const \
(katetextcursor.h:134) ==10276==    by 0x62F9035: \
Kate::TextBlock::updateRange(Kate::TextRange*) (katetextblock.cpp:572) ==10276==    \
by 0x62F8C57: Kate::TextBlock::mergeBlock(Kate::TextBlock*) (katetextblock.cpp:522) \
==10276==    by 0x62EFAA1: Kate::TextBuffer::balanceBlock(int) \
(katetextbuffer.cpp:494) ==10276==    by 0x62EF1B2: Kate::TextBuffer::unwrapLine(int) \
(katetextbuffer.cpp:298) ==10276==    by 0x637BB9C: KateBuffer::unwrapLines(int, int) \
(katebuffer.cpp:292) ==10276==    by 0x6359DE9: KateDocument::editRemoveLines(int, \
int) (katedocument.cpp:1321) ==10276==    by 0x6359AD6: \
KateDocument::editRemoveLine(int) (katedocument.cpp:1293) ==10276==    by 0x6357841: \
KateDocument::removeLine(int) (katedocument.cpp:732) ==10276==    by 0x6394C6A: \
KateScriptDocument::removeLine(int) (katescriptdocument.cpp:513) ==10276==    by \
0x62EAE77: KateScriptDocument::qt_static_metacall(QObject*, QMetaObject::Call, int, \
void**) (moc_katescriptdocument.cpp:299) ==10276==    by 0x62EC69D: \
KateScriptDocument::qt_metacall(QMetaObject::Call, int, void**) \
(moc_katescriptdocument.cpp:468) ==10276==  Address 0x14353470 is 96 bytes inside a \
block of size 152 free'd ==10276==    at 0x4C2A44B: operator delete(void*) (in \
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==10276==    by 0x62FDE4D: \
Kate::TextRange::~TextRange() (katetextrange.cpp:66) ==10276==    by 0x6470B1E: \
KateOnTheFlyChecker::deleteMovingRange(KTextEditor::MovingRange*) \
(ontheflycheck.cpp:535) ==10276==    by 0x6470657: \
KateOnTheFlyChecker::rangeEmpty(KTextEditor::MovingRange*) (ontheflycheck.cpp:489) \
==10276==    by 0x62FE641: Kate::TextRange::checkValidity(int, int, bool) \
(katetextrange.cpp:196) ==10276==    by 0x62F6C84: \
Kate::TextBlock::wrapLine(KTextEditor::Cursor const&) (katetextblock.cpp:168) \
==10276==    by 0x62EEF1D: Kate::TextBuffer::wrapLine(KTextEditor::Cursor const&) \
(katetextbuffer.cpp:231) ==10276==    by 0x637BA6D: \
KateBuffer::wrapLine(KTextEditor::Cursor const&) (katebuffer.cpp:275) ==10276==    by \
0x635972E: KateDocument::editInsertLine(int, QString const&) (katedocument.cpp:1246) \
==10276==    by 0x6357690: KateDocument::insertLine(int, QString const&) \
(katedocument.cpp:706) ==10276==    by 0x6394C32: KateScriptDocument::insertLine(int, \
QString const&) (katescriptdocument.cpp:508) ==10276==    by 0x62EAE28: \
KateScriptDocument::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) \
(moc_katescriptdocument.cpp:297)

From the lower half of this valgrind trace:
In Kate's on-the-fly spell-checker, there is this peace of code:

  void KateOnTheFlyChecker::rangeEmpty(KTextEditor::MovingRange *range)
  {
    deleteMovingRange (range);
  }
and:
  void KateOnTheFlyChecker::deleteMovingRange(KTextEditor::MovingRange *range)
  {
    // bla bla
    delete(range);
  }

So the range is deleted because it got empty. Ok, especially since the api doc says:
"You may delete the range inside this method"

In the Kate::TextRange destructor it says:

  // remove range from m_ranges
1 fixLookup (m_start.line(), m_end.line(), -1, -1);
  
  // remove this range from the buffer
2 m_buffer.m_ranges.remove (this);

To 1:
It removes this range from the corresponding TextBlocks (in this case just one),
since the new position is (-1, -1).

Kate::TextBlock::removeRange(TextRange* r) removes r from
- the m_uncachedRanges set, and 
- the hash m_cachedRangesForLine and
- the vector m_cachedRangesForLine

To 2:
Then the destructor removes the deleted TextRange from the (global) m_ranges list.

In theory, we're now all good, and the pointer should be nowhere registered.
Correct?


Now to the upper half of the valgrind trace: TextBlock::mergeBlock line 519: 

  QList<TextRange*> allRanges = m_uncachedRanges.toList() + \
m_cachedLineForRanges.keys();  foreach(TextRange* range, allRanges) {
    removeRange(range);
    targetBlock->updateRange(range);
  }

As I understand, something is broken with the range in updateRange(range).
Reading "m_block" in the Range's TextCursor is invalid. Thus, the TextCursor
must be invalid. Thus, the Range must be invalid.

So what's going on? The TextRange is still in one of the cached lists,
although it shouldn't be.

How is that?
Deleting a TextRange removes all pointers. However, in this case, through
the MovingRangeFeedback class, the range is deleted in the middle of wrapping
a line in Kate::TextBuffer::wrapLine (katetextbuffer.cpp:231).

Might it be that this is somehow a short period of an inconsistent state?
Some line mismatch? There somewhere must be an off-by-one... Or maybe not ;)

Would be interesting what happens, if the on-the-fly spellchecker would
delete the range with a small delay. I bet the crash is gone then.

Greetings,
Dominik
_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel


[prev in list] [next in list] [prev in thread] [next in thread]