Fix crash on clicking "Show documentation for xyz" in code tooltip
ClosedPublic

Authored by kossebau on Jun 29 2017, 3:37 PM.

Details

Summary

Sync handling of focus change on opening the documentation docker
results in destruction of AbstractNavigationWidget object
and thus also the AbstractNavigationContext object it holds,
which is a QSharedData subclass, this way crashing the
QExplicitlySharedDataPointer on itself returned from the method
once it gets to its destructor.

Test Plan

No longer crashes when documentation docker is closed and the
"Show documentation for xyz" link is activated.

Diff Detail

Repository
R33 KDevPlatform
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Jun 29 2017, 3:37 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJun 29 2017, 3:37 PM

See "Show documentation for QWidget" at bottom of tooltip if unsure what kind of link this is about:

apol accepted this revision.Jun 29 2017, 5:21 PM
apol added a subscriber: apol.

Ugh, I've seen this issue. Fine by me if it's what it takes...

language/duchain/navigation/abstractnavigationcontext.cpp
218

maybe Q_DECLARE_METATYPE in idocumentation.h?

This revision is now accepted and ready to land.Jun 29 2017, 5:21 PM

Thanks for review!

language/duchain/navigation/abstractnavigationcontext.cpp
218

There is already a Q_DECLARE_METATYPE(KDevelop::IDocumentation::Ptr), but seems that does not work, there was a runtime error about the type not being known when I relied on that initially.
API dox of qRegisterMetaType mentions that it is needed for typedefs and QueuedConnections, so might be the case here as well (using Ptr = QExplicitlySharedDataPointer<IDocumentation>; inside IDocumentation class declaration).
Not sure if this was perhaps due to no KDevelop:: prefix used with the slot argument type?
I stopped experimenting when I read the API dox comment (and when things worked ;) ).
Some expert reading this who could lift the curtain on the details?

This revision was automatically updated to reflect the committed changes.
mwolff added a subscriber: mwolff.Jun 30 2017, 12:12 PM

could you move the register metatype please?

language/duchain/navigation/abstractnavigationcontext.cpp
218

this should be done in a central place, once. or at least not every time the action is invoked - so maybe put it into a ctor

we should really clean that up and introduce a central method for the individual libs to register their metatypes...

kossebau marked an inline comment as done.Jun 30 2017, 5:21 PM
kossebau added inline comments.
language/duchain/navigation/abstractnavigationcontext.cpp
218

Moved to constructor with 6055d62d28df8f6da47a77d1e3f066a14c973db3 (also for two other existing call instances)