Commit a947074f authored by Jonathan L. Verner's avatar Jonathan L. Verner Committed by Igor Kushnir
Browse files

Fix a crash in the "update signature action".

The problem seems to be that the DUChain is readlocked in
`AbstractNavigationWidgetPrivate::anchorClicked` (see also [2]),
which then proceeds through the following (backtrace-like) call chain

  #9  AdaptSignatureAction::execute()
      (at plugins/clang/codegen/adaptsignatureaction.cpp:83)
  #10 ProblemNavigationContext::executeAction(int)
(at kdevplatform/language/duchain/navigation
problemnavigationcontext.cpp:258)
  #11 ProblemNavigationContext::executeKeyAction(QString const&)
(at
kdevplatform/language/duchain/navigation/problemnavigationcontext.cpp:243)
const&)
(at
kdevplatform/language/duchain/navigation/abstractnavigationcontext.cpp:183)
  #13 AbstractNavigationContext::acceptLink(QString const&)
(at
kdevplatform/language/duchain/navigation/abstractnavigationcontext.cpp:487)
  #14 AbstractNavigationWidgetPrivate::anchorClicked

which ends at

  plugins/clang/codegen/adaptsignatureaction.cpp:83

with an `ENSURE_CHAIN_NOT_LOCKED` macro, which asserts.

However, the lock in `anchorClicked` was added there in
commit ff72bc32 to fix bug 386901 ([1]) so it cannot
just be removed. The callchain triggering the 386901 bug looks
as follows:

  #0 FunctionDefinition::declaration
     (at kdevplatform/language/duchain/functiondefinition.cpp:52)
  #1 FunctionDefinition::declaration
     (at kdevplatform/language/duchain/functiondefinition.cpp:52)
AbstractDeclarationNavigationContext::AbstractDeclarationNavigationContext
(at
kdevplatform/language/duchain/navigation/abstractdeclarationnavigationcontext.cpp:67)
  #3 DeclarationNavigationContext::AbstractDeclarationNavigationContext
     (at plugins/clang/duchain/navigationwidget.cpp:38)
  #4 ClangNavigationWidget::ClangNavigationWidget
     (at plugins/clang/duchain/navigationwidget.cpp:98)
  #5 ClangDUContext<KDevelop::TopDUContext, 140>::createNavigationWidget
     (at plugins/clang/duchain/clangducontext.cpp:46)
  #6 AbstractNavigationContext::registerChild
(at
kdevplatform/language/duchain/navigation/abstractnavigationcontext.cpp:281)
  #7 AbstractNavigationContext::execute
(at
kdevplatform/language/duchain/navigation/abstractnavigationcontext.cpp:201)
  #8 AbstractNavigationContext::acceptLink
(at
kdevplatform/language/duchain/navigation/abstractnavigationcontext.cpp:487)
  #9 AbstractNavigationWidgetPrivate::anchorClicked
(at
kdevplatform/language/duchain/navigation/abstractnavigationwidget.cpp:285)

which hits an assert at

   kdevplatform/language/duchain/functiondefinition.cpp:52

in the `ENSURE_CAN_READ` macro.

This commit moves the lock from `anchorClicked` into
`AbstractNavigationContext::registerChild`, which is the
last opportunity for a lock before a language-plugin specific
method is called (so that the bug does not reappear in
other language plugins).

References

[1] https://bugs.kde.org/show_bug.cgi?id=386901
[2] https://phabricator.kde.org/D22182

BUG: 416714
CCBUG: 358787
FIXED-IN: 5.8.220401
parent 46925d9b
......@@ -264,6 +264,17 @@ NavigationContextPointer AbstractNavigationContext::registerChild(const Declarat
{
Q_D(AbstractNavigationContext);
// Lock the DUChain for reading, since the createNavigationWidget
// method will eventually need read access to the DUChain and
// does not lock it itself.
DUChainReadLocker lock;
// Check that declaration and its context are still valid
// (they might have been deleted while we were waiting for the lock)
if (!declaration || !declaration->context()) {
return NavigationContextPointer(this);
}
//We create a navigation-widget here, and steal its context.. evil ;)
QScopedPointer<AbstractNavigationWidget> navigationWidget(
declaration->context()->createNavigationWidget(declaration.data()));
......
......@@ -266,8 +266,6 @@ void AbstractNavigationWidget::navigateDeclaration(const IndexedDeclaration& dec
void AbstractNavigationWidgetPrivate::anchorClicked(const QUrl& url)
{
DUChainReadLocker lock;
//We may get deleted while the call to acceptLink, so make sure we don't crash in that case
QPointer<AbstractNavigationWidget> thisPtr(q);
NavigationContextPointer nextContext = m_context->acceptLink(url.toString());
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment