Skip to content

WIP: omp-jb53964#472

Open
sergey-levin wants to merge 2 commits intosailfishos:masterfrom
sergey-levin:omp-jb53964
Open

WIP: omp-jb53964#472
sergey-levin wants to merge 2 commits intosailfishos:masterfrom
sergey-levin:omp-jb53964

Conversation

@sergey-levin
Copy link
Copy Markdown
Contributor

Connect to ensureBuildEngineRuns using AutoConnection will start nested event loop inside main event loop.
That may break code execution inside CMakeBuildSystem::triggerParsing if user choose to disable current kit.
The kit will be deleted inside nested event loop, then code execution return to CMakeBuildSystem::triggerParsing.
This will lead to dangling pointers (for example inside projectDirectory call) and eventually QtC will crash.
Using QueuedConnection in the contrary prevents breaking code execution and therefore should solve the problem.

*/
connect(target, &Target::parsingStarted,
this, &MerCMakeBuildConfiguration::ensureBuildEngineRuns);
this, &MerCMakeBuildConfiguration::ensureBuildEngineRuns, Qt::QueuedConnection);
Copy link
Copy Markdown
Member

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

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.

@martyone
Copy link
Copy Markdown
Member

BTW, we will soon have our side upgraded to upstream v4.15.2, so maybe wait for that.

@sergey-levin sergey-levin changed the title omp-jb53964 WIP: omp-jb53964 Sep 16, 2021
@sergey-levin
Copy link
Copy Markdown
Contributor Author

BTW, we will soon have our side upgraded to upstream v4.15.2, so maybe wait for that.

Thanks for the info. I will check related sources in upstream 4.15.2 in case it will be decided to edit cmake plugin code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants