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 <orgads@gmail.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
This commit is contained in:
Jarek Kobus
2023-02-02 23:32:26 +01:00
parent ce0f48e0f9
commit 202b696677
6 changed files with 20 additions and 11 deletions

View File

@@ -66,8 +66,9 @@ public:
bool isCanceled() const { return m_watcher.isCanceled(); } bool isCanceled() const { return m_watcher.isCanceled(); }
QFuture<ResultType> future() const { return m_watcher.future(); } QFuture<ResultType> future() const { return m_watcher.future(); }
ResultType result() const { return m_watcher.result(); } // TODO: warn when isRunning? ResultType result() const { return m_watcher.result(); }
QList<ResultType> results() const { return m_watcher.future().results(); } QList<ResultType> results() const { return future().results(); }
bool isResultAvailable() const { return future().resultCount(); }
private: private:
StartHandler m_startHandler; StartHandler m_startHandler;

View File

@@ -67,6 +67,7 @@ void CppProjectUpdater::update(const ProjectUpdateInfo &projectUpdateInfo,
async.setFutureSynchronizer(&m_futureSynchronizer); async.setFutureSynchronizer(&m_futureSynchronizer);
}; };
const auto onInfoGeneratorDone = [=](const AsyncTask<ProjectInfo::ConstPtr> &async) { const auto onInfoGeneratorDone = [=](const AsyncTask<ProjectInfo::ConstPtr> &async) {
if (async.isResultAvailable())
storage->projectInfo = async.result(); storage->projectInfo = async.result();
}; };
QList<TaskItem> tasks{parallel}; QList<TaskItem> tasks{parallel};

View File

@@ -109,16 +109,17 @@ DiffFilesController::DiffFilesController(IDocument *document)
setDisplayName(Tr::tr("Diff")); setDisplayName(Tr::tr("Diff"));
using namespace Tasking; using namespace Tasking;
const TreeStorage<QList<FileData>> storage; const TreeStorage<QList<std::optional<FileData>>> storage;
const auto setupTree = [this, storage](TaskTree &taskTree) { const auto setupTree = [this, storage](TaskTree &taskTree) {
QList<FileData> *outputList = storage.activeStorage(); QList<std::optional<FileData>> *outputList = storage.activeStorage();
const auto setupDiff = [this](AsyncTask<FileData> &async, const ReloadInput &reloadInput) { const auto setupDiff = [this](AsyncTask<FileData> &async, const ReloadInput &reloadInput) {
async.setAsyncCallData(DiffFile(ignoreWhitespace(), contextLineCount()), reloadInput); async.setAsyncCallData(DiffFile(ignoreWhitespace(), contextLineCount()), reloadInput);
async.setFutureSynchronizer(Internal::DiffEditorPlugin::futureSynchronizer()); async.setFutureSynchronizer(Internal::DiffEditorPlugin::futureSynchronizer());
}; };
const auto onDiffDone = [outputList](const AsyncTask<FileData> &async, int i) { const auto onDiffDone = [outputList](const AsyncTask<FileData> &async, int i) {
if (async.isResultAvailable())
(*outputList)[i] = async.result(); (*outputList)[i] = async.result();
}; };
@@ -126,7 +127,7 @@ DiffFilesController::DiffFilesController(IDocument *document)
outputList->resize(inputList.size()); outputList->resize(inputList.size());
using namespace std::placeholders; using namespace std::placeholders;
QList<TaskItem> tasks {parallel, continueOnDone}; QList<TaskItem> tasks {parallel, optional};
for (int i = 0; i < inputList.size(); ++i) { for (int i = 0; i < inputList.size(); ++i) {
tasks.append(Async<FileData>(std::bind(setupDiff, _1, inputList.at(i)), tasks.append(Async<FileData>(std::bind(setupDiff, _1, inputList.at(i)),
std::bind(onDiffDone, _1, i))); std::bind(onDiffDone, _1, i)));
@@ -134,7 +135,13 @@ DiffFilesController::DiffFilesController(IDocument *document)
taskTree.setupRoot(tasks); taskTree.setupRoot(tasks);
}; };
const auto onTreeDone = [this, storage] { const auto onTreeDone = [this, storage] {
setDiffFiles(*storage.activeStorage()); const QList<std::optional<FileData>> &results = *storage.activeStorage();
QList<FileData> finalList;
for (const std::optional<FileData> &result : results) {
if (result.has_value())
finalList.append(*result);
}
setDiffFiles(finalList);
}; };
const auto onTreeError = [this, storage] { const auto onTreeError = [this, storage] {
setDiffFiles({}); setDiffFiles({});

View File

@@ -874,7 +874,7 @@ void SideBySideDiffEditorWidget::showDiff()
m_controller.setBusyShowing(true); m_controller.setBusyShowing(true);
connect(m_asyncTask.get(), &AsyncTaskBase::done, this, [this] { connect(m_asyncTask.get(), &AsyncTaskBase::done, this, [this] {
if (m_asyncTask->isCanceled()) { if (m_asyncTask->isCanceled() || !m_asyncTask->isResultAvailable()) {
for (SideDiffEditorWidget *editor : m_editor) for (SideDiffEditorWidget *editor : m_editor)
editor->clearAll(Tr::tr("Retrieving data failed.")); editor->clearAll(Tr::tr("Retrieving data failed."));
} else { } else {

View File

@@ -458,7 +458,7 @@ void UnifiedDiffEditorWidget::showDiff()
m_asyncTask->setFutureSynchronizer(DiffEditorPlugin::futureSynchronizer()); m_asyncTask->setFutureSynchronizer(DiffEditorPlugin::futureSynchronizer());
m_controller.setBusyShowing(true); m_controller.setBusyShowing(true);
connect(m_asyncTask.get(), &AsyncTaskBase::done, this, [this] { 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.")); setPlainText(Tr::tr("Retrieving data failed."));
} else { } else {
const ShowResult result = m_asyncTask->result(); const ShowResult result = m_asyncTask->result();

View File

@@ -340,7 +340,7 @@ Tasking::TaskItem ProcessExtraCompiler::taskItemImpl(const ContentProvider &prov
async.setFutureSynchronizer(futureSynchronizer()); async.setFutureSynchronizer(futureSynchronizer());
}; };
const auto taskDone = [=](const AsyncTask<FileNameToContentsHash> &async) { const auto taskDone = [=](const AsyncTask<FileNameToContentsHash> &async) {
if (async.results().size() == 0) if (!async.isResultAvailable())
return; return;
const FileNameToContentsHash data = async.result(); const FileNameToContentsHash data = async.result();
if (data.isEmpty()) if (data.isEmpty())