From 202b696677c13079dd503d0f814aecdc64bfa00b Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Thu, 2 Feb 2023 23:32:26 +0100 Subject: [PATCH] DiffEditor: Fix a crash when "No difference" It may happen that async task associated with diffing one pair of files may not report any value. This happens when both file contents are the same. In this case taking a result from async task on a successful done will lead to crash. Add AsyncTask::isResultAvailable() method. Use it in client code just in case, where needed. Fix DiffFilesController, so that no result is allowed for all running tasks (i.e. make the main group optional). Collect list of optional results instead of direct results. The empty optional on the list means the result wasn't delivered by async task and it's skipped. Fixes: QTCREATORBUG-28750 Change-Id: I4ca678a187fad619bae470da3e806e8c8da61127 Reviewed-by: Orgad Shaneh Reviewed-by: Qt CI Bot --- src/libs/utils/asynctask.h | 5 +++-- src/plugins/cppeditor/cppprojectupdater.cpp | 3 ++- src/plugins/diffeditor/diffeditorplugin.cpp | 17 ++++++++++++----- .../diffeditor/sidebysidediffeditorwidget.cpp | 2 +- .../diffeditor/unifieddiffeditorwidget.cpp | 2 +- src/plugins/projectexplorer/extracompiler.cpp | 2 +- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/libs/utils/asynctask.h b/src/libs/utils/asynctask.h index bb537cef168..e035977401b 100644 --- a/src/libs/utils/asynctask.h +++ b/src/libs/utils/asynctask.h @@ -66,8 +66,9 @@ public: bool isCanceled() const { return m_watcher.isCanceled(); } QFuture future() const { return m_watcher.future(); } - ResultType result() const { return m_watcher.result(); } // TODO: warn when isRunning? - QList results() const { return m_watcher.future().results(); } + ResultType result() const { return m_watcher.result(); } + QList results() const { return future().results(); } + bool isResultAvailable() const { return future().resultCount(); } private: StartHandler m_startHandler; diff --git a/src/plugins/cppeditor/cppprojectupdater.cpp b/src/plugins/cppeditor/cppprojectupdater.cpp index 91bcd943b12..a784d3bb794 100644 --- a/src/plugins/cppeditor/cppprojectupdater.cpp +++ b/src/plugins/cppeditor/cppprojectupdater.cpp @@ -67,7 +67,8 @@ void CppProjectUpdater::update(const ProjectUpdateInfo &projectUpdateInfo, async.setFutureSynchronizer(&m_futureSynchronizer); }; const auto onInfoGeneratorDone = [=](const AsyncTask &async) { - storage->projectInfo = async.result(); + if (async.isResultAvailable()) + storage->projectInfo = async.result(); }; QList tasks{parallel}; tasks.append(Async(setupInfoGenerator, onInfoGeneratorDone)); diff --git a/src/plugins/diffeditor/diffeditorplugin.cpp b/src/plugins/diffeditor/diffeditorplugin.cpp index e18a0e27972..2a813206657 100644 --- a/src/plugins/diffeditor/diffeditorplugin.cpp +++ b/src/plugins/diffeditor/diffeditorplugin.cpp @@ -109,24 +109,25 @@ DiffFilesController::DiffFilesController(IDocument *document) setDisplayName(Tr::tr("Diff")); using namespace Tasking; - const TreeStorage> storage; + const TreeStorage>> storage; const auto setupTree = [this, storage](TaskTree &taskTree) { - QList *outputList = storage.activeStorage(); + QList> *outputList = storage.activeStorage(); const auto setupDiff = [this](AsyncTask &async, const ReloadInput &reloadInput) { async.setAsyncCallData(DiffFile(ignoreWhitespace(), contextLineCount()), reloadInput); async.setFutureSynchronizer(Internal::DiffEditorPlugin::futureSynchronizer()); }; const auto onDiffDone = [outputList](const AsyncTask &async, int i) { - (*outputList)[i] = async.result(); + if (async.isResultAvailable()) + (*outputList)[i] = async.result(); }; const QList inputList = reloadInputList(); outputList->resize(inputList.size()); using namespace std::placeholders; - QList tasks {parallel, continueOnDone}; + QList tasks {parallel, optional}; for (int i = 0; i < inputList.size(); ++i) { tasks.append(Async(std::bind(setupDiff, _1, inputList.at(i)), std::bind(onDiffDone, _1, i))); @@ -134,7 +135,13 @@ DiffFilesController::DiffFilesController(IDocument *document) taskTree.setupRoot(tasks); }; const auto onTreeDone = [this, storage] { - setDiffFiles(*storage.activeStorage()); + const QList> &results = *storage.activeStorage(); + QList finalList; + for (const std::optional &result : results) { + if (result.has_value()) + finalList.append(*result); + } + setDiffFiles(finalList); }; const auto onTreeError = [this, storage] { setDiffFiles({}); diff --git a/src/plugins/diffeditor/sidebysidediffeditorwidget.cpp b/src/plugins/diffeditor/sidebysidediffeditorwidget.cpp index 5bc887b90aa..da69e607840 100644 --- a/src/plugins/diffeditor/sidebysidediffeditorwidget.cpp +++ b/src/plugins/diffeditor/sidebysidediffeditorwidget.cpp @@ -874,7 +874,7 @@ void SideBySideDiffEditorWidget::showDiff() m_controller.setBusyShowing(true); connect(m_asyncTask.get(), &AsyncTaskBase::done, this, [this] { - if (m_asyncTask->isCanceled()) { + if (m_asyncTask->isCanceled() || !m_asyncTask->isResultAvailable()) { for (SideDiffEditorWidget *editor : m_editor) editor->clearAll(Tr::tr("Retrieving data failed.")); } else { diff --git a/src/plugins/diffeditor/unifieddiffeditorwidget.cpp b/src/plugins/diffeditor/unifieddiffeditorwidget.cpp index 8c8bee05c27..b937b919f7c 100644 --- a/src/plugins/diffeditor/unifieddiffeditorwidget.cpp +++ b/src/plugins/diffeditor/unifieddiffeditorwidget.cpp @@ -458,7 +458,7 @@ void UnifiedDiffEditorWidget::showDiff() m_asyncTask->setFutureSynchronizer(DiffEditorPlugin::futureSynchronizer()); m_controller.setBusyShowing(true); connect(m_asyncTask.get(), &AsyncTaskBase::done, this, [this] { - if (m_asyncTask->isCanceled()) { + if (m_asyncTask->isCanceled() || !m_asyncTask->isResultAvailable()) { setPlainText(Tr::tr("Retrieving data failed.")); } else { const ShowResult result = m_asyncTask->result(); diff --git a/src/plugins/projectexplorer/extracompiler.cpp b/src/plugins/projectexplorer/extracompiler.cpp index ff7c061790e..ab72d3608ff 100644 --- a/src/plugins/projectexplorer/extracompiler.cpp +++ b/src/plugins/projectexplorer/extracompiler.cpp @@ -340,7 +340,7 @@ Tasking::TaskItem ProcessExtraCompiler::taskItemImpl(const ContentProvider &prov async.setFutureSynchronizer(futureSynchronizer()); }; const auto taskDone = [=](const AsyncTask &async) { - if (async.results().size() == 0) + if (!async.isResultAvailable()) return; const FileNameToContentsHash data = async.result(); if (data.isEmpty())