1. 26 Oct, 2020 1 commit
    • Milian Wolff's avatar
      Fix version check for GDB 10.x · 8831937f
      Milian Wolff authored
      Don't try to parse a version via a regex, just QVersionNumber
      instead. While at it, port to QRegularExpression and don't
      crash if the response list is empty.
  2. 24 Oct, 2020 1 commit
  3. 23 Oct, 2020 1 commit
    • Christoph Roick's avatar
      Do not block "Run All Tests" by dialog · a48287f4
      Christoph Roick authored
      - always start a new test job without asking to kill running jobs
        * disable the combo box in the launch config interface
      By default, app jobs are compared by their executable name. If a new job
      name matches an old job, a dialog pops up to ask whether the old instance
      shall be killed. This blocks the "Run All Tests" function under some
      circumstances (for instance when "cmake" is used to invoke the test).
      The patch always makes a CTest-generated job run without asking the user.
      BUG: 377639
      FIXED-IN: 5.6.1
  4. 18 Oct, 2020 2 commits
    • Igor Kushnir's avatar
      Optimize StatusbarProgressWidget::connectSingleItem() · ff1c26d4
      Igor Kushnir authored
      For each progress value report StatusBar::showProgress() calls
      ProgressItem::setUsesBusyIndicator(), which eventually results in a call
      to StatusbarProgressWidget::connectSingleItem(). The return value of
      ProgressManager::instance()->singleItem() changes very rarely between
      these calls. Let us avoid reconnecting the same signal/slot pair
    • Igor Kushnir's avatar
      Don't show a busy indicator for 5s after a task completion · 0d87cab9
      Igor Kushnir authored
      81220db1 eliminated such misleading
      progress reports for items that use a busy indicator themselves, but
      missed the case when a quick percent-progress-reporting task follows
      right after a busy-indicator task.
      For example, GrepJob's WorkCollectFiles step uses a busy indicator, then
      its next WorkGrep step reports progress in percents. So when the
      WorkGrep step is completed in less than a second (e.g. when there are
      not many files to search in and the looked-for string is not present in
      any of them), the bottom-right-corner progress bar shows
      a busy indicator for 5 seconds.
      Let us display 100% progress during these 5 seconds instead.
  5. 17 Oct, 2020 1 commit
  6. 16 Oct, 2020 1 commit
    • Igor Kushnir's avatar
      Fix a new test_documentcontroller test failure · 60ffbc7c
      Igor Kushnir authored
      300ea014 introduced
      DocumentControllerPrivate::shuttingDown variable. It is set to false in
      constructor and to true in DocumentController::cleanup(). However
      TestDocumentController::init() calls DocumentController::initialize()
      before each test function; TestDocumentController::cleanup() calls
      DocumentController::cleanup() after each test function. So the second
      test function fails to open a document after shutdown.
      Let us set DocumentControllerPrivate::shuttingDown to false in
      DocumentController::initialize() to fix the test failure.
  7. 15 Oct, 2020 6 commits
    • Igor Kushnir's avatar
      Do not open documents after DocumentController::cleanup() · 300ea014
      Igor Kushnir authored
      For now d->shuttingDown is checked in just one of many
      DocumentController::openDocument() overloads and alternatives, because
      this is the only overload that is known to be called after cleanup() and
      cause crashes. If similar crashes happen because of other ways of
      opening a document, the check can be moved or reused elsewhere.
      Or perhaps a different strategy will be needed to fix these hypothetical
      future crashes: e.g. simply returning nullptr doesn't work if the caller
      dereferences the returned document pointer without checking.
      BUG: 425994
      FIXED-IN: 5.6.1
    • Igor Kushnir's avatar
      MIDebugger: check ICore::debugController() for nullptr · 79c2b0f6
      Igor Kushnir authored
      When a user exits KDevelop while debugging a program, a queued call to
      MIDebugger::readyReadStandardOutput() may be invoked during or after
      ~CorePrivate(). If this happens after ~DebugController() (which is the
      parent of BreakpointModel), a crash is likely, because
      readyReadStandardOutput() indirectly calls
      MIBreakpointController::updateFromDebugger(), which assumes that
      BreakpointModel is not null (as do all other MIBreakpointController's
      member functions).
      Note that MIDebugger::readyReadStandardOutput() after ~DebugController()
      can be invoked both from inside ~MIDebugger() (more precisely, from
      inside the QProcess::waitForFinished() call) and before the destructor.
      There is no need to process a debugger's standard output and risk a
      crash when KDevelop has almost finished shutting down.
      BUG: 425993
      FIXED-IN: 5.6.1
    • Igor Kushnir's avatar
      Destroy not started debug sessions · 6a4f671d
      Igor Kushnir authored
      When a debug session is stopped via MIDebugSession::stopDebugger() and
      discarded while the s_dbgNotStarted state is still on, the session is
      never destroyed. This is a genuine resource leak because the number of
      such unused but not destroyed debug sessions is unlimited.
      The leak can be easily reproduced by triggering Debug Launch with a
      nonexistent executable specified in the current launch configuration.
      A more difficult way to reproduce the leak is to Debug-Launch many times
      in quick succession: some sessions will never reach
      MIDebugSession::startDebugging() and debug-print the following line:
      kdevelop.plugins.debuggercommon: Stopping debugger when it's not started
      The leak is already detected in MIDebugSession::stopDebugger() when the
      aforementioned message is debug-printed. Let us set the session state to
      EndedState there. This session state change is connected to
      DebugController::debuggerStateChanged(), which calls deleteLater() on
      the debug session.
    • Igor Kushnir's avatar
      Safely finalize and destroy DebugSession objects · 37ff587e
      Igor Kushnir authored
      DebugController is responsible for destroying debug sessions and it can
      not do so past its own destructor. Currently, depending on the relative
      timing of KDevelop exit and the 5 second interval of the singleshot
      timer in MIDebugSession::stopDebugger(), a DebugSession may be destroyed
      safely in time or not destroyed at all and possibly cause a crash by
      accessing the already destroyed DebugController or its children.
      Let us kill debugger processes and thus finalize the debug sessions'
      states in ~DebugController(). We delay the killing of debugger processes
      for as long as possible to give them a chance to exit on their own.
      Check ICore::documentController() for nullptr in
      BreakpointModel::updateMarks() and
      DebugController::clearExecutionPoint() to prevent crashes when these
      slots are invoked by DebugSession's state transition signals from
      inside ~DebugController(), which is called after ~DocumentController().
      BUG: 425991
      FIXED-IN: 5.6.1
    • Igor Kushnir's avatar
      Check ICore::activeSession() for nullptr in BreakpointModel::save() · c60e8303
      Igor Kushnir authored
      When a user exits KDevelop while debugging a program, a queued call to
      BreakpointModel::save() may be invoked during Core::cleanup() or perhaps
      even during ~CorePrivate(). If this happens between
      SessionController::cleanup() and ~DebugController() (which is the parent
      of BreakpointModel), the result is a crash. A breakpoint change that is
      saved at this time is likely unimportant and can be safely skipped.
      BUG: 425986
      FIXED-IN: 5.6.1
    • Igor Kushnir's avatar
      MIDebugSession::m_plugin: raw pointer => QPointer · 89ffddc7
      Igor Kushnir authored
      When a user exits KDevelop during or shortly after debugging a program,
      a DebugSession object may outlive MIDebuggerPlugin. This can cause a
      crash in ~DebugSession(). Both GDB::DebugSession and LLDB::DebugSession
      already support nullptr m_plugin for testing purposes, so this QPointer
      type change alone eliminates the crash.
      BUG: 425985
      FIXED-IN: 5.6.1
  8. 14 Oct, 2020 1 commit
    • Bernd Buschinski's avatar
      Fix memory leak in QuickOpenWidget · 9d9eb66b
      Bernd Buschinski authored
      - Qt Doc: "The event is not deleted when the event has been sent.
                 The normal approach is to create the event on the stack..."
      - Found by clang 10 static analyzer
  9. 12 Oct, 2020 2 commits
  10. 11 Oct, 2020 2 commits
  11. 07 Oct, 2020 19 commits
    • Igor Kushnir's avatar
      Don't delay Stop and Tool Views action menus · 85747fda
      Igor Kushnir authored
      For many years I thought that KDevelop's Stop toolbar menu was broken.
      Today after digging into the relevant code and documentation I found out
      that one has to press and hold the button for a few seconds for the
      submenu to show up.
      From the documentation to KActionMenu::setDelayed():
        Remember that if the "main" action (the toolbar button itself) cannot
        be clicked, then you should call setDelayed(false).
      This is clearly the case for the Stop menu button and for the Tool Views
      menu button (which can be added to the Main Toolbar through Configure
      Toolbars dialog): clicking these buttons has no effect.
    • Igor Kushnir's avatar
      Don't invoke BackgroundParser::parseComplete() twice on failure · 5b2f9881
      Igor Kushnir authored
      When a job fails, ThreadWeaver::QObjectDecorator emits both failed() and
      done() signals. BackgroundParser::parseComplete() is connected to both.
      So when a ParseJob fails, parseComplete() is called twice for the same
      job. The most obvious consequence is the too great value of
      Two classes derived from ParseJob - CMakeParseJob and QmlJsParseJob -
      call ParseJob::abortJob() when they are aborted, which makes
      ParseJob::success() return false and thus causes QObjectDecorator to
      emit both signals.
      In order to test what happens when the bug is triggered, I edited
      CMakeParseJob::run() code to unconditionally call `abortJob(); return;`
      after the `if (!package.isEmpty()) {` line. KDevelop seems to handle
      this well, but there are lots of relevant debug messages in the log:
      kdevplatform.language: m_doneParseJobs larger than m_maxParseJobs: 0 -1
      kdevplatform.language: m_doneParseJobs larger than m_maxParseJobs: 1 0
      The simple fix is to connect only to the
      &ThreadWeaver::QObjectDecorator::done signal. Its documentation states:
      "This signal is emitted when the job has been finished (no matter if it
      succeeded or not)."
    • Igor Kushnir's avatar
      Kill CTestFindJob when its project is destroyed · 103ab0be
      Igor Kushnir authored
      When a user closes a project while its associated CTestFindJob is still
      working, the job is not notified and keeps running. After that, when
      BackgroundParser eventually parses all documents requested by the job,
      the job calls TestController::addTestSuite(). Inside this call the
      CTestSuite::project() pointer is dereferenced in
      TestView::addTestSuite() => TestView::itemForProject(), which causes a
      segmentation fault.
      Ideally all associated CTestFindJob-s should be killed just before or
      just after a project is closed to revert their no longer useful requests
      to BackgroundParser as soon as possible. However, reliably detecting
      that the project is no longer open is not straightforward, especially if
      the project opening is aborted or the project is closed even before a
      CTestFindJob is created. My implementation of such a proper fix is
      intertwined with several other CTest-, TestController- and
      TestView-related fixes. The combined fix will be so large that it won't
      fit into the 5.6 branch. So this commit implements a simple temporary
      workaround for the fairly easy to trigger crash: check if the project is
      destroyed in CTestFindJob::updateReady() and kill the job if this is the
      BUG: 329246
      FIXED-IN: 5.6.1
    • Igor Kushnir's avatar
      Early-return from CTestFindJob::updateReady() if the job is finished · ae9f45ae
      Igor Kushnir authored
      When a user exits KDevelop while a CTestFindJob is still working, the
      job is killed from RunController::cleanup() and its KJob parent calls
      deleteLater(). But the killed job can be still not destroyed after
      DUChain::shutdown() is called, which results in a crash if
      CTestFindJob::updateReady() is then invoked via Qt::QueuedConnection.
      Note that ~ParseJob() queues calls to CTestFindJob::updateReady(),
      which can then be invoked after DUChain::shutdown(), if ParseJob-s are
      running when the user exits KDevelop and no event loop is entered in the
      time between the ~ParseJob() and DUChain::shutdown() calls (sometimes
      the optional QCoreApplication::processEvents() call in
      BackgroundParser::waitForIdle() intervenes and prevents the crash).
      Core::cleanup() calls backgroundParser()->waitForIdle(), which ensures
      that all parse jobs finish, are destroyed and queue "updateReady" calls.
      BUG: 423651
      FIXED-IN: 5.6.1
    • Igor Kushnir's avatar
      Optimize open-file-only path of ParseProjectJob::queueFilesToParse() · 62a336eb
      Igor Kushnir authored
      Let the more efficient loop below queue each set element instead of
      searching for each open document in the set gradually emptying it.
    • Igor Kushnir's avatar
      ParseProjectJob: force-update open documents too · 4a219966
      Igor Kushnir authored
      When ParseProjectJob's forceUpdate argument is true, closed documents
      are parsed with TopDUContext::ForceUpdate feature flag. I think that not
      taking into account the forceUpdate argument when parsing open documents
      is an omission. Let us parse all documents with the
      TopDUContext::ForceUpdate feature flag when forceUpdate is true.
    • Igor Kushnir's avatar
      Make two ParseProjectJobPrivate's data members const · 0249d7aa
      Igor Kushnir authored
    • Igor Kushnir's avatar
      ParseProjectJob: QSet::find => QSet::constFind · 6cae00af
      Igor Kushnir authored
      This prevents unnecessary detaching.
    • Igor Kushnir's avatar
      ParseProjectJob: revert all requests in doKill(), not destructor · 5490b12c
      Igor Kushnir authored
      ParseProjectJob finishes normally when all its requests are satisfied by
      BackgroundParser, in which case there is nothing to revert. Both
      ProjectController and RunController abort parsing by killing the job.
      Apart from not doing unnecessary work when ParseProjectJob finishes
      normally, this commit prevents a hypothetical crash in case the
      BackgroundParser's destruction precedes a ParseProjectJob's destruction.
    • Igor Kushnir's avatar
      ParseProjectJob: don't skip parsing the last few files · f0464333
      Igor Kushnir authored
      ParseProjectJob::queueFilesToParse() removes the currently open
      documents that belong to the project from d->filesToParse. Therefore,
      when ParseProjectJob::updateReady() is invoked, the actual number of
      files to parse can be greater than d->filesToParse.size().
      ParseProjectJob reverts all its requests in the destructor, so the few
      unaccounted-for closed documents could remain unparsed.
    • Igor Kushnir's avatar
      Remove empty ParseProjectJob::updateProgress() · e93f0c85
      Igor Kushnir authored
      Only RunController listens to ParseProjectJob::percent() signal (as it
      does to any registered job's percent() signal) in order to update total
      progress. Background Parser already reports parsing progress, so there
      is no need to emit ParseProjectJob::percent(), and so it makes sense to
      remove the empty updateProgress() function rather than implement it.
    • Igor Kushnir's avatar
      ParseProjectJob: don't depend on IProjectController · f054f4da
      Igor Kushnir authored
      Both the code and the documentation of ParseProjectJob are simplified by
      letting ProjectController call its parseAllProjectSources() function.
    • Igor Kushnir's avatar
      ParseProjectJob: don't unregister with RunController redundantly · 960a39e9
      Igor Kushnir authored
      KJob::finished() signal is guaranteed to be emitted from ~KJob() or
      earlier. RunController::finished() slot is connected to this signal and
      unregisters the job that emits it.
    • Igor Kushnir's avatar
      Kill ParseProjectJob before closing its project · 0b037a8c
      Igor Kushnir authored
      Remove the now redundant deleting of the ParseProjectJob when its
      project is destroyed. ParseProjectJob objects are created in
      ProjectController::reparseProject(), which is called when the project is
      already fully open. So the project is guaranteed to be closed before it
      is destroyed.
      A Project is destroyed via deleteLater() after it is closed, so if the
      event loop is busy at the time, &IProject::destroyed can be emitted much
      later than IProjectController::projectClosing(). With this commit
      ParseProjectJob is killed earlier than it was destroyed without the
      Early-return from ParseProjectJob::queueFilesToParse() not only when the
      job has been destroyed, but when it has been killed too. The earlier
      return not just avoids unnecessary work, but is essential during the
      application shutdown: prevents a crash by not accessing IndexedString
      after DUChain::shutdown(). Note that KJob::kill() calls deleteLater(),
      so a job can be destroyed a long time after it is killed if the event
      loop is busy, as it is at shutdown. ParseProjectJob is killed early in
      the shutdown process from RunController::cleanup() - before
      ProjectController::cleanup(), which would kill it otherwise, and
      (importantly to prevent the crash) before DUChain::shutdown().
      Remove the shuttingDown() check from ParseProjectJob::start(), because
      this member function doesn't access globals on its own. The appropriate
      safety checks are now performed in the scheduled
      Don't call deleteLater() in ParseProjectJob::doKill() just before
      returning true: rely on auto-delete KJob base class to call it.
      BUG: 427387
      FIXED-IN: 5.6.1
    • Igor Kushnir's avatar
      Don't process events in ParseProjectJob::start() · 89077c7c
      Igor Kushnir authored
      From KJob::start() documentation:
      Warning: Never implement any synchronous workload in this method.
      This method should just trigger the job startup, not do any work itself.
      It is expected to be non-blocking.
      When a user exits KDevelop in the ParseProjectJob::start()'s nested
      event loop, RunController may be destroyed in the time between
      `job->start();` and `checkState();` statements in
      RunController::registerJob(). The result is a segmentation fault in
      BUG: 427386
      FIXED-IN: 5.6.1
    • Igor Kushnir's avatar
      Remove the ParseProjectJob pointer when a project is closed · 87d28ba2
      Igor Kushnir authored
      Storing the QPointer-s indefinitely was a small memory leak.
      Eliminate a redundant QHash search in
      Fix ProjectController::reparseProject() indentation.
    • Igor Kushnir's avatar
      Remove unused ParseProjectJobPrivate::project variable · 4f0ea7cb
      Igor Kushnir authored
    • Script Kiddy's avatar
      SVN_SILENT made messages (.desktop file) - always resolve ours · 3211fc2a
      Script Kiddy authored
      In case of conflict in i18n, keep the version of the branch "ours"
      To resolve a particular conflict, "git checkout --ours path/to/file.desktop"
    • David Redondo's avatar
      Do not crash when disabling problem reporter plugin · 4c535989
      David Redondo authored
      If we do no unregister ourselves, KTextEditor will try calling into the
      destroyed notes provider.
  12. 22 Sep, 2020 1 commit
    • Igor Kushnir's avatar
      Don't crash after Cancel in Job Already Running dialog · 147add2e
      Igor Kushnir authored
      When a user Execute-Launches an application a second time while the
      previously launched instance of this application is still running, the
      "Job Already Running" dialog appears with 3 buttons. If a user clicks
      the Cancel button, a NativeAppJob kills itself Quietly. This
      NativeAppJob belongs to an ExecuteCompositeJob, which is not notified
      when a subjob is killed Quietly. So the ExecuteCompositeJob keeps
      waiting for it to finish. When this waiting ExecuteCompositeJob is
      killed (e.g. via "Stop All" button or on KDevelop exit), it attempts to
      kill the subjob it believes is still running, but which in fact is long
      since destroyed. This usually causes a segmentation fault.
      Aleix Pol fixed a similar crash when the "Kill All Instances" button in
      the "Job Already Running" dialog is clicked in
      BUG: 399511, 416874
      FIXED-IN: 5.6.1
  13. 10 Sep, 2020 2 commits