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

Fix qgis server fcgi response destructor #59632

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

benoitdm-oslandia
Copy link
Contributor

Suppress wait in fcgi response destructor

In the previous version, the response destructor waited for the monitoring thread to end that may induce a 333ms sleep each time. We fix that by using a classic ptr to separate the thread and the response lifecycles.

@benoitdm-oslandia benoitdm-oslandia self-assigned this Nov 28, 2024
@benoitdm-oslandia benoitdm-oslandia added the Server Related to QGIS server label Nov 28, 2024
@github-actions github-actions bot added this to the 3.42.0 milestone Nov 28, 2024
Copy link

github-actions bot commented Nov 28, 2024

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 61588b8)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 61588b8)

src/server/qgsfcgiserverresponse.h Outdated Show resolved Hide resolved
src/server/qgsfcgiserverresponse.h Outdated Show resolved Hide resolved
@benoitdm-oslandia benoitdm-oslandia force-pushed the fix/fcgi_response_destructor branch from 97ebe44 to fc27b8d Compare November 28, 2024 15:11
@benoitdm-oslandia benoitdm-oslandia marked this pull request as draft November 29, 2024 13:43
@benoitdm-oslandia benoitdm-oslandia force-pushed the fix/fcgi_response_destructor branch 2 times, most recently from 8a62b11 to fcebbdc Compare December 6, 2024 16:18
Copy link
Contributor

@troopa81 troopa81 left a comment

Choose a reason for hiding this comment

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

I prefer way much this solution that the one(s) before, thanks @benoitdm-oslandia 👍

Just a few comments

@benoitdm-oslandia benoitdm-oslandia force-pushed the fix/fcgi_response_destructor branch 3 times, most recently from 1def89d to 00b994a Compare December 17, 2024 14:21
@benoitdm-oslandia benoitdm-oslandia marked this pull request as ready for review December 17, 2024 14:21
@benoitdm-oslandia
Copy link
Contributor Author

@troopa81 @elpaso this is ready! If you can make a final review please!

@benoitdm-oslandia benoitdm-oslandia marked this pull request as draft December 19, 2024 09:49
@benoitdm-oslandia benoitdm-oslandia force-pushed the fix/fcgi_response_destructor branch from dfd986e to bf5724e Compare December 19, 2024 10:16
@benoitdm-oslandia benoitdm-oslandia force-pushed the fix/fcgi_response_destructor branch from 86dbb51 to 6dcd431 Compare December 20, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Server Related to QGIS server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants