From 47b04b4ff3e886353f39da9f335d6ceef0b05fac 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 by using timed_mutex --- src/server/qgsfcgiserverresponse.cpp | 84 +++++++++++++++++++++------- src/server/qgsfcgiserverresponse.h | 39 +++++++++---- 2 files changed, 91 insertions(+), 32 deletions(-) diff --git a/src/server/qgsfcgiserverresponse.cpp b/src/server/qgsfcgiserverresponse.cpp index cef9fce77241..0411777545f6 100644 --- a/src/server/qgsfcgiserverresponse.cpp +++ b/src/server/qgsfcgiserverresponse.cpp @@ -27,8 +27,10 @@ #include "qgslogger.h" #if defined( Q_OS_UNIX ) && !defined( Q_OS_ANDROID ) +#include #include #include +#include // // QgsFCGXStreamData copied from libfcgi FCGX_Stream_Data @@ -54,37 +56,57 @@ typedef struct QgsFCGXStreamData } QgsFCGXStreamData; #endif -QgsSocketMonitoringThread::QgsSocketMonitoringThread( bool *isResponseFinished, QgsFeedback *feedback ) - : mIsResponseFinished( isResponseFinished ) - , mFeedback( feedback ) +// to be able to use 333ms expression as a duration +using namespace std::chrono_literals; + + +// QgsSocketMonitoringThread constructor +QgsSocketMonitoringThread::QgsSocketMonitoringThread( std::shared_ptr feedback ) + : mFeedback( feedback ) , mIpcFd( -1 ) { - setObjectName( "FCGI socket monitor" ); - Q_ASSERT( mIsResponseFinished ); Q_ASSERT( mFeedback ); + mShouldStop.store( false ); + #if defined( Q_OS_UNIX ) && !defined( Q_OS_ANDROID ) if ( FCGI_stdout && FCGI_stdout->fcgx_stream && FCGI_stdout->fcgx_stream->data ) { - QgsFCGXStreamData *stream = static_cast( FCGI_stdin->fcgx_stream->data ); + QgsFCGXStreamData *stream = static_cast( FCGI_stdout->fcgx_stream->data ); if ( stream && stream->reqDataPtr ) { mIpcFd = stream->reqDataPtr->ipcFd; } else { - QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdin stream data is null! Socket monitoring disable." ), QStringLiteral( "FCGIServer" ), Qgis::MessageLevel::Warning ); + QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdout stream data is null! Socket monitoring disabled." ), // + QStringLiteral( "FCGIServer" ), // + Qgis::MessageLevel::Warning ); } } else { - QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdin is null! Socket monitoring disable." ), QStringLiteral( "FCGIServer" ), Qgis::MessageLevel::Warning ); + QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdout is null! Socket monitoring disabled." ), // + QStringLiteral( "FCGIServer" ), // + Qgis::MessageLevel::Warning ); } #endif } +// Informs the thread to quit +void QgsSocketMonitoringThread::stop() +{ + mShouldStop.store( true ); + // Release the mutex so the try_lock in the thread will not wait anymore and + // the thread will end its loop as we have set 'mShouldStop' to true + mMutex.unlock(); +} + void QgsSocketMonitoringThread::run() { + // Lock the thread mutex: every try_lock will take 333ms + mMutex.lock(); + if ( mIpcFd < 0 ) { QgsMessageLog::logMessage( QStringLiteral( "Socket monitoring disabled: no socket fd!" ), QStringLiteral( "FCGIServer" ), Qgis::MessageLevel::Warning ); @@ -92,33 +114,49 @@ void QgsSocketMonitoringThread::run() } #if defined( Q_OS_UNIX ) && !defined( Q_OS_ANDROID ) +#ifdef QGISDEBUG + const pid_t threadId = gettid(); +#endif + + mShouldStop.store( false ); char c; - while ( !*mIsResponseFinished ) + while ( !mShouldStop.load() ) { const ssize_t x = recv( mIpcFd, &c, 1, MSG_PEEK | MSG_DONTWAIT ); // see https://stackoverflow.com/a/12402596 - if ( x < 0 ) + if ( x != 0 ) { // Ie. we are still connected but we have an 'error' as there is nothing to read - QgsDebugMsgLevel( QStringLiteral( "FCGIServer: remote socket still connected. errno: %1" ).arg( errno ), 5 ); + QgsDebugMsgLevel( QStringLiteral( "FCGIServer %1: remote socket still connected. errno: %2, x: %3" ) // + .arg( threadId ) + .arg( errno ) + .arg( x ), + 5 ); } else { // socket closed, nothing can be read - QgsDebugMsgLevel( QStringLiteral( "FCGIServer: remote socket has been closed! errno: %1" ).arg( errno ), 2 ); + QgsDebugMsgLevel( QStringLiteral( "FCGIServer %1: remote socket has been closed! errno: %2, x: %3" ) // + .arg( threadId ) + .arg( errno ) + .arg( x ), + 1 ); mFeedback->cancel(); break; } - QThread::msleep( 333L ); + // If lock is acquired this means the response has finished and we will exit the while loop + // else we will wait max for 333ms. + if ( mMutex.try_lock_for( 333ms ) ) + mMutex.unlock(); } - if ( *mIsResponseFinished ) + if ( mShouldStop.load() ) { - QgsDebugMsgLevel( QStringLiteral( "FCGIServer: socket monitoring quits normally." ), 2 ); + QgsDebugMsgLevel( QStringLiteral( "FCGIServer::run %1: socket monitoring quits normally." ).arg( threadId ), 2 ); } else { - QgsDebugMsgLevel( QStringLiteral( "FCGIServer: socket monitoring quits: no more socket." ), 2 ); + QgsDebugMsgLevel( QStringLiteral( "FCGIServer::run %1: socket monitoring quits: no more socket." ).arg( threadId ), 1 ); } #endif } @@ -134,15 +172,21 @@ QgsFcgiServerResponse::QgsFcgiServerResponse( QgsServerRequest::Method method ) mBuffer.open( QIODevice::ReadWrite ); setDefaultHeaders(); - mSocketMonitoringThread = std::make_unique( &mFinished, mFeedback.get() ); - mSocketMonitoringThread->start(); + mSocketMonitoringThread = std::make_unique( mFeedback ); + + // Start the monitoring thread + mThread = std::thread( &QgsSocketMonitoringThread::run, mSocketMonitoringThread.get() ); } QgsFcgiServerResponse::~QgsFcgiServerResponse() { mFinished = true; - mSocketMonitoringThread->exit(); - mSocketMonitoringThread->wait(); + + // Inform the thread to quit asap + mSocketMonitoringThread->stop(); + + // Just to be sure + mThread.join(); } void QgsFcgiServerResponse::removeHeader( const QString &key ) diff --git a/src/server/qgsfcgiserverresponse.h b/src/server/qgsfcgiserverresponse.h index 17e54a7e3400..8d7e8f116227 100644 --- a/src/server/qgsfcgiserverresponse.h +++ b/src/server/qgsfcgiserverresponse.h @@ -26,7 +26,8 @@ #include "qgsserverresponse.h" #include -#include +#include +#include /** * \ingroup server @@ -34,23 +35,32 @@ * \brief Thread used to monitor the fcgi socket * \since QGIS 3.36 */ -class QgsSocketMonitoringThread : public QThread +class QgsSocketMonitoringThread { - Q_OBJECT - public: /** - * \brief QgsSocketMonitoringThread - * \param isResponseFinished - * \param feedback + * Constructor for QgsSocketMonitoringThread + * \param feedback used to cancel rendering jobs when socket timedout + */ + QgsSocketMonitoringThread( std::shared_ptr feedback ); + + /** + * main thread function */ - QgsSocketMonitoringThread( bool *isResponseFinished, QgsFeedback *feedback ); void run(); + /** + * Stop the thread + */ + void stop(); + private: - bool *mIsResponseFinished = nullptr; - QgsFeedback *mFeedback = nullptr; + std::atomic_bool mShouldStop; + std::shared_ptr mFeedback; int mIpcFd = -1; + + // used to synchronize socket monitoring thread and fcgi response + std::timed_mutex mMutex; }; /** @@ -66,7 +76,8 @@ class SERVER_EXPORT QgsFcgiServerResponse : public QgsServerResponse * \param method The HTTP method (Get by default) */ QgsFcgiServerResponse( QgsServerRequest::Method method = QgsServerRequest::GetMethod ); - virtual ~QgsFcgiServerResponse(); + + virtual ~QgsFcgiServerResponse() override; void setHeader( const QString &key, const QString &value ) override; @@ -115,8 +126,12 @@ class SERVER_EXPORT QgsFcgiServerResponse : public QgsServerResponse QgsServerRequest::Method mMethod; int mStatusCode = 0; + // encapsulate thread data std::unique_ptr mSocketMonitoringThread; - std::unique_ptr mFeedback; + // real thread object. Used to join. + std::thread mThread; + // Used to cancel rendering jobs + std::shared_ptr mFeedback; }; #endif