Skip to content
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.

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

Avoid deadlock in QGIS server #46701

Merged
merged 1 commit into from Jan 20, 2022
Merged

Avoid deadlock in QGIS server #46701

merged 1 commit into from Jan 20, 2022

Conversation

m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented Jan 5, 2022

In case of invalid map settings, renderJob.start() returns immediately and the finished() signal is emitted before the connection happens. This puts the server into an eternal loop.

void QgsMapRendererJob::start()
{
if ( mSettings.hasValidSettings() )
startPrivate();
else
{
mErrors.append( QgsMapRendererJob::Error( QString(), tr( "Invalid map settings" ) ) );
emit finished();
}
}

@github-actions github-actions bot added this to the 3.24.0 milestone Jan 5, 2022
@elpaso elpaso added Server Related to QGIS server Bug Either a bug report, or a bug fix. Let's hope for the latter! labels Jan 5, 2022
@elpaso
Copy link
Contributor

elpaso commented Jan 5, 2022

backport?

@m-kuhn
Copy link
Member Author

m-kuhn commented Jan 5, 2022

likely, not yet verified

@m-kuhn
Copy link
Member Author

m-kuhn commented Jan 5, 2022

Probably introduced in #43376
3.16 is not affected

@rldhont
Copy link
Contributor

rldhont commented Jan 5, 2022

Thanks @m-kuhn

@dmarteau a review ?

@rldhont
Copy link
Contributor

rldhont commented Jan 5, 2022

Same error in release3-16. Is backport to 3.16 possible ?

@m-kuhn
Copy link
Member Author

m-kuhn commented Jan 5, 2022

How did you test?

@m-kuhn
Copy link
Member Author

m-kuhn commented Jan 8, 2022

@rldhont still interested in a reprod steps, this could help to add a test

@pblottiere
Copy link
Member

Thanks @m-kuhn! And +1 for adding a test.

@nyalldawson
Copy link
Collaborator

3.16 is EOL, so the discussion is moot now

@m-kuhn
Copy link
Member Author

m-kuhn commented Jan 19, 2022

I would propose to merge this now.
We can still add a test later if we have a reprod feedback from @rldhont or someone else

@m-kuhn m-kuhn closed this Jan 20, 2022
@m-kuhn m-kuhn reopened this Jan 20, 2022
@m-kuhn m-kuhn enabled auto-merge January 20, 2022 20:51
@m-kuhn m-kuhn merged commit 1c8f9d9 into qgis:master Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption Server Related to QGIS server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants