Review Request: Fix for debugger crash and memory leak

[Thomas Schöps]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3619/
-----------------------------------------------------------

(Updated 2010-04-16 10:06:28.843046)


Review request for KDevelop.


Changes
-------

New try (not much left):

- removed changes to Variable::setInScope()
- removed recursive deletion of items in TreeItem::deleteChildren()

Like Milian wrote, it looks like the TreeItem destructor doesn't do the deletion correctly, but I don't want to make another wrong suggestion ...

The original crash was a one-time issue, I sadly can't reproduce it.


Summary
-------

As the debugger is rather unstable for me and I think it is important to improve this before the 4.0.0 release, I tried to debug it a bit. I had a look at this crash:

Thread 1 (Thread 0xb76d4700 (LWP 4615)):
[KCrash Handler]
#6  0x008be7bc in __dynamic_cast () from /usr/lib/libstdc++.so.6
#7  0x07a5982c in KDevelop::Variable::setInScope (this=0xa7db178, v=true) at /home/thomas/Data/kdevplatform/debugger/variable/variablecollection.cpp:103
#8  0x025ff505 in KDevelop::GdbVariable::handleUpdate (this=0xa7db178, var=...) at /home/thomas/Data/kdevelop/debuggers/gdb/gdbvariable.cpp:267
#9  0x025facae in GDBDebugger::VariableController::handleVarUpdate (this=0xa728b88, r=...) at /home/thomas/Data/kdevelop/debuggers/gdb/variablecontroller.cpp:94
#10 0x025cf83c in GDBDebugger::GDBCommand::invokeHandler (this=0xd1366e8, r=...) at /home/thomas/Data/kdevelop/debuggers/gdb/gdbcommand.cpp:76
#11 0x025c7e08 in GDBDebugger::GDB::processLine (this=0xccab8e8, line=...) at /home/thomas/Data/kdevelop/debuggers/gdb/gdb.cpp:278
#12 0x025c8a2e in GDBDebugger::GDB::readyReadStandardOutput (this=0xccab8e8) at /home/thomas/Data/kdevelop/debuggers/gdb/gdb.cpp:175
#13 0x025c4eb3 in GDBDebugger::GDB::qt_metacall (this=0xccab8e8, _c=QMetaObject::InvokeMetaMethod, _id=15, _a=0xbfeb648c) at /home/thomas/Data/kdevelop/build/debuggers/gdb/moc_gdb.cpp:105
#14 0x0506c263 in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib/libQtCore.so.4
...

The problem seems to be that KDevelop::Variable::setInScope assumes that all its children can be casted to Variable*, but there can be an ellipsis item as the last one which is of type EllipsisItem and not Variable.
I tested it: setInScope is called with more_ being true (meaning that there is an ellipsis item). So this code will fail, because childCount() includes the ellipsis item and child() returns it:

for (int i=0; i < childCount(); ++i) {
    if (Variable *var = dynamic_cast<Variable*>(child(i))) {
        var->setInScope(v);
    }
}

Iterating over childItems excludes the ellipsis item.

While looking for the cause, I also had a look at TreeItem::deleteChildren(). The "copy" vector is currently unused there, and because clear() calls childItems.clear(), the child items won't be deleted in the following loop. Apparently the author wanted to use the copy for deleting. Also, IVariableController::stateChanged calls this method only on the first level of Locals while removing them, so I think it should recursively delete all children instead of only the first level. I've added the recursive deletion in the patch.


Diffs (updated)
-----

  trunk/extragear/sdk/kdevplatform/debugger/util/treeitem.cpp 1115078 

Diff: http://reviewboard.kde.org/r/3619/diff


Testing
-------


Thanks,

Thomas