From kwrite-devel Sat Mar 16 17:38:29 2013 From: Dominik Haumann Date: Sat, 16 Mar 2013 17:38:29 +0000 To: kwrite-devel Subject: deletion of TextRange Message-Id: <1399310.LOBd6YD02K () eriador> X-MARC-Message: https://marc.info/?l=kwrite-devel&m=136345552832042 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 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