Skip to content

WIP: omp-jb53964 #472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sergey-levin wants to merge 2 commits into
base: master
Choose a base branch
from
Open

WIP: omp-jb53964 #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3e4a51
Mer: Fix dangling target pointer
sergey-levin Jun 4, 2021
a00021c
Mer: Fix segmentation fault when disabling active kit
sergey-levin Sep 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/plugins/mer/mercmakebuildconfiguration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,13 @@ MerCMakeBuildConfiguration::MerCMakeBuildConfiguration(Target *target, Utils::Id
connect(aspect, &BaseAspect::changed,
this, &BuildConfiguration::updateCacheAndEmitEnvironmentChanged);

/*
* We want to be sure that BE is running before parsing starts.
* We should not start BE while parsing is in progress using AutoConnection - it lead to crash during kit disable.
* Instead we start BE on parsingStarted signal using QueuedConnection.
*/
connect(target, &Target::parsingStarted,
this, &MerCMakeBuildConfiguration::ensureBuildEngineRuns);
this, &MerCMakeBuildConfiguration::ensureBuildEngineRuns, Qt::QueuedConnection);
Copy link
Copy Markdown
Member

@martyone martyone Sep 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code had the effect that the emitting side (where emit parsingStarted() is called) was blocked for the time of starting the build engine (which may involve dialog with the user who may or may not permit to start the build engine). With queued connection the emitting side continues immediately I believe, which is not wanted.

Now I checked the original PR and yeah I already mentioned the issue there.

We need to find some other solution, possibly with changes to cmake plugin, so that we can delay and possibly abort parsing depending on build engine readiness.

Copy link
Copy Markdown
Contributor Author

@sergey-levin sergey-levin Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which may involve dialog with the user who may or may not permit to start the build engine

I did not notice that (maybe because I checked something like "always start VM and don't ask again" recently).

With queued connection the emitting side continues immediately I believe, which is not wanted.

I will bring back start VM dialog and double check this.

possibly with changes to cmake plugin

This condition may help, because I intentionally didn't touch QtC code while tried to fix this issue.

}

void MerCMakeBuildConfiguration::doInitialize(const ProjectExplorer::BuildInfo &info)
Expand Down
1 change: 1 addition & 0 deletions src/plugins/mer/merdeployconfiguration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ void MerAddRemoveSpecialDeployStepsProjectListener::timerEvent(QTimerEvent *even
{
if (event->timerId() == m_updateTargetsTimer.timerId()) {
m_updateTargetsTimer.stop();
m_updateTargetsQueue.removeAll(nullptr);
while (!m_updateTargetsQueue.isEmpty())
updateTarget(m_updateTargetsQueue.dequeue());
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/mer/merdeployconfiguration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private slots:
static void removeStep(ProjectExplorer::BuildStepList *stepList, Utils::Id stepId);

private:
QQueue<ProjectExplorer::Target *> m_updateTargetsQueue;
QQueue<QPointer<ProjectExplorer::Target>> m_updateTargetsQueue;
QBasicTimer m_updateTargetsTimer;
};

Expand Down