KDE/kdevplatform/debugger/interfaces

[Niko Sams]

On Sat, Nov 28, 2009 at 12:58, Vladimir Prus <ghost at cs.msu.su> wrote:
> On Saturday 28 November 2009 14:52:00 Niko Sams wrote:
>
>> SVN commit 1055628 by nsams:
>>
>> Update locals only once. (fixes crash)
>>
>>  M  +34 -9     ivariablecontroller.cpp
>>  M  +14 -2     ivariablecontroller.h
>>
>>
>> --- trunk/KDE/kdevplatform/debugger/interfaces/ivariablecontroller.cpp #1055627:1055628
>> @@ -24,13 +24,16 @@
>>  #include "../../interfaces/icore.h"
>>  #include "../../interfaces/idebugcontroller.h"
>>  #include "../variable/variablecollection.h"
>> +#include "iframestackmodel.h"
>>
>>  namespace KDevelop {
>>
>>
>>  IVariableController::IVariableController(IDebugSession* parent)
>> -    : QObject(parent)
>> +    : QObject(parent), m_activeThread(-1), m_activeFrame(-1)
>>  {
>> +    connect(parent, SIGNAL(stateChanged(KDevelop::IDebugSession::DebuggerState)),
>> +            SLOT(stateChanged(KDevelop::IDebugSession::DebuggerState)));
>>  }
>>
>>  VariableCollection* IVariableController::variableCollection()
>> @@ -38,11 +41,18 @@
>>      return ICore::self()->debugController()->variableCollection();
>>  }
>>
>> -void IVariableController::handleEvent(IDebugSession::event_t event)
>> +IDebugSession* IVariableController::session() const
>>  {
>> -    switch (event) {
>> -    case IDebugSession::program_exited:
>> -    case IDebugSession::debugger_exited:
>> +    return static_cast<IDebugSession*>(parent());
>> +}
>> +
>> +void IVariableController::stateChanged(IDebugSession::DebuggerState state)
>> +{
>> +    if (state == IDebugSession::ActiveState) {
>> +        //variables are now outdated, update them
>> +        m_activeThread = -1;
>> +        m_activeFrame = -1;
>> +    } else if (state == IDebugSession::StoppedState) {
>>          // Remove all locals.
>>          foreach (Locals *l, variableCollection()->allLocals()) {
>>              l->deleteChildren();
>> @@ -55,17 +65,32 @@
>>                  var->setInScope(false);
>>              }
>>          }
>> -        break;
>> +    }
>> +}
>>
>> +void IVariableController::updateIfFrameOrThreadChanged()
>> +{
>> +    IFrameStackModel *sm = session()->frameStackModel();
>> +    if (sm->currentThread() != m_activeThread || sm->currentFrame() != m_activeFrame) {
>> +        m_activeThread = sm->currentThread();
>> +        m_activeFrame = sm->currentFrame();
>> +        update();
>> +    }
>> +}
>
first: thanks for having an eye on my commits.

> Where was the crash?
the assertion in GdbVariable::setVarobj Q_ASSERT(varobj_.isEmpty())
triggered, because
two -create-varobj commands where queued. And that was caused by two
-stack-list-locals, each
calling GdbVariable::attachMaybe.

> Can it still happen if we quickly switch frames?
hmmm probably. I will write an unit test for this.

> And why is thread_or_frame changed event emitted when no change has
> happened -- is that probably what needs such a check?
That was my first go too. thread_or_frame_changed is raised in
FrameStackModel::setCurrentThread
as this is true:
bool changed = (threadNumber != m_currentThread) || (m_currentFrame != 0);
changin it to:
bool changed = (m_currentThread != -1 && threadNumber !=
m_currentThread) || (m_currentFrame!=-1 && m_currentFrame != 0);
fixes the problem too.
However I thought it might be incorrect. Thinking about it, it might
be correct; the frame didn't change,
it was the first frame we got. What do you think?

Niko