From fc27b8dcbba9d9cb78922ec2bd4440f5ec19c639 Mon Sep 17 00:00:00 2001 From: bdm-oslandia Date: Tue, 26 Nov 2024 07:26:38 +0100 Subject: [PATCH] fix(qgisserver): 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. --- src/server/qgsfcgiserverresponse.cpp | 53 ++++++++++++++++++++++------ src/server/qgsfcgiserverresponse.h | 21 ++++++++--- 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/server/qgsfcgiserverresponse.cpp b/src/server/qgsfcgiserverresponse.cpp index 7a61e34ef50d..a70e4eb59670 100644 --- a/src/server/qgsfcgiserverresponse.cpp +++ b/src/server/qgsfcgiserverresponse.cpp @@ -54,13 +54,11 @@ typedef struct QgsFCGXStreamData } QgsFCGXStreamData; #endif -QgsSocketMonitoringThread::QgsSocketMonitoringThread( bool *isResponseFinished, QgsFeedback *feedback ) - : mIsResponseFinished( isResponseFinished ) - , mFeedback( feedback ) +QgsSocketMonitoringThread::QgsSocketMonitoringThread( QgsFeedback *feedback ) + : mFeedback( feedback ) , mIpcFd( -1 ) { setObjectName( "FCGI socket monitor" ); - Q_ASSERT( mIsResponseFinished ); Q_ASSERT( mFeedback ); #if defined(Q_OS_UNIX) && !defined(Q_OS_ANDROID) @@ -73,20 +71,29 @@ QgsSocketMonitoringThread::QgsSocketMonitoringThread( bool *isResponseFinished, } else { - QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdin stream data is null! Socket monitoring disable." ), + QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdin stream data is null! Socket monitoring disabled." ), QStringLiteral( "FCGIServer" ), Qgis::MessageLevel::Warning ); } } else { - QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdin is null! Socket monitoring disable." ), + QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdin is null! Socket monitoring disabled." ), QStringLiteral( "FCGIServer" ), Qgis::MessageLevel::Warning ); } + + // Suicide the thread when it ends + connect( this, &QThread::finished, this, &QThread::deleteLater ); + #endif } +void QgsSocketMonitoringThread::setResponseFinished( bool responseFinished ) +{ + mIsResponseFinished = responseFinished; +} + void QgsSocketMonitoringThread::run( ) { if ( mIpcFd < 0 ) @@ -99,7 +106,7 @@ void QgsSocketMonitoringThread::run( ) #if defined(Q_OS_UNIX) && !defined(Q_OS_ANDROID) char c; - while ( !*mIsResponseFinished ) + while ( !mIsResponseFinished ) { const ssize_t x = recv( mIpcFd, &c, 1, MSG_PEEK | MSG_DONTWAIT ); // see https://stackoverflow.com/a/12402596 if ( x < 0 ) @@ -118,7 +125,7 @@ void QgsSocketMonitoringThread::run( ) QThread::msleep( 333L ); } - if ( *mIsResponseFinished ) + if ( mIsResponseFinished ) { QgsDebugMsgLevel( QStringLiteral( "FCGIServer: socket monitoring quits normally." ), 2 ); } @@ -141,15 +148,39 @@ QgsFcgiServerResponse::QgsFcgiServerResponse( QgsServerRequest::Method method ) mBuffer.open( QIODevice::ReadWrite ); setDefaultHeaders(); - mSocketMonitoringThread = std::make_unique( &mFinished, mFeedback.get() ); + // This is not a unique_ptr because we want the response to not depend on the thread lifecycle. + mSocketMonitoringThread = new QgsSocketMonitoringThread( mFeedback.get() ); mSocketMonitoringThread->start(); } +QgsFcgiServerResponse::QgsFcgiServerResponse( const QgsFcgiServerResponse © ) + : mFinished( copy.mFinished ) + , mHeadersSent( copy.mHeadersSent ) + , mMethod( copy.mMethod ) + , mStatusCode( copy.mStatusCode ) + , mSocketMonitoringThread( nullptr ) + , mFeedback( nullptr ) +{ +} + +QgsFcgiServerResponse &QgsFcgiServerResponse::operator =( const QgsFcgiServerResponse © ) +{ + mFinished = copy.mFinished; + mHeadersSent = copy.mHeadersSent; + mMethod = copy.mMethod; + mStatusCode = copy.mStatusCode; + mSocketMonitoringThread = nullptr; + mFeedback = nullptr; + + return *this; +} + QgsFcgiServerResponse::~QgsFcgiServerResponse() { mFinished = true; - mSocketMonitoringThread->exit(); - mSocketMonitoringThread->wait(); + if ( mSocketMonitoringThread ) + // This will allow the thread to finish sleeping and exit its while loop in the background, without us needing to wait for it to finish. + mSocketMonitoringThread->setResponseFinished( mFinished ); } void QgsFcgiServerResponse::removeHeader( const QString &key ) diff --git a/src/server/qgsfcgiserverresponse.h b/src/server/qgsfcgiserverresponse.h index 25e6025d3bf3..859a41291b1d 100644 --- a/src/server/qgsfcgiserverresponse.h +++ b/src/server/qgsfcgiserverresponse.h @@ -45,11 +45,13 @@ class QgsSocketMonitoringThread: public QThread * \param isResponseFinished * \param feedback */ - QgsSocketMonitoringThread( bool *isResponseFinished, QgsFeedback *feedback ); + QgsSocketMonitoringThread( QgsFeedback *feedback ); void run( ); + void setResponseFinished( bool responseFinished ); + private: - bool *mIsResponseFinished = nullptr; + bool mIsResponseFinished = false; QgsFeedback *mFeedback = nullptr; int mIpcFd = -1; }; @@ -68,7 +70,18 @@ class SERVER_EXPORT QgsFcgiServerResponse: public QgsServerResponse * \param method The HTTP method (Get by default) */ QgsFcgiServerResponse( QgsServerRequest::Method method = QgsServerRequest::GetMethod ); - virtual ~QgsFcgiServerResponse(); + + /** + * Dummy copy constructor. Needed because of QgsSocketMonitoringThread ptr + */ + QgsFcgiServerResponse( const QgsFcgiServerResponse © ); + + /** + * Dummy operator = . Needed because of QgsSocketMonitoringThread ptr + */ + QgsFcgiServerResponse &operator = ( const QgsFcgiServerResponse © ); + + virtual ~QgsFcgiServerResponse() override; void setHeader( const QString &key, const QString &value ) override; @@ -117,7 +130,7 @@ class SERVER_EXPORT QgsFcgiServerResponse: public QgsServerResponse QgsServerRequest::Method mMethod; int mStatusCode = 0; - std::unique_ptr mSocketMonitoringThread; + QgsSocketMonitoringThread *mSocketMonitoringThread; std::unique_ptr mFeedback; };