From fcebbdc6585a56886308c852b46400f0dcd02649 Mon Sep 17 00:00:00 2001
From: bdm-oslandia <benoit.de.mezzo@oslandia.com>
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 | 98 +++++++++++++++++++++-------
src/server/qgsfcgiserverresponse.h | 38 +++++++----
2 files changed, 100 insertions(+), 36 deletions(-)
diff --git a/src/server/qgsfcgiserverresponse.cpp b/src/server/qgsfcgiserverresponse.cpp
index e0d031885cf4..c71befad5cf7 100644
--- a/src/server/qgsfcgiserverresponse.cpp
+++ b/src/server/qgsfcgiserverresponse.cpp
@@ -26,6 +26,10 @@
#include "qgslogger.h"
+#include <unistd.h>
+#include <mutex>
+#include <chrono>
+
#if defined(Q_OS_UNIX) && !defined(Q_OS_ANDROID)
#include <sys/types.h>
#include <sys/socket.h>
@@ -54,41 +58,54 @@ 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;
+
+// used to synchronize socket monitoring thread and fcgi response
+std::timed_mutex gSocketMonitoringMutex;
+
+
+// QgsSocketMonitoringThread constructor
+QgsSocketMonitoringThread::QgsSocketMonitoringThread( std::shared_ptr<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)
if ( FCGI_stdout && FCGI_stdout->fcgx_stream && FCGI_stdout->fcgx_stream->data )
{
- QgsFCGXStreamData *stream = static_cast<QgsFCGXStreamData *>( FCGI_stdin->fcgx_stream->data );
+ QgsFCGXStreamData *stream = static_cast<QgsFCGXStreamData *>( 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." ),
+ 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." ),
+ QgsMessageLog::logMessage( QStringLiteral( "FCGI_stdout is null! Socket monitoring disabled." ),
QStringLiteral( "FCGIServer" ),
Qgis::MessageLevel::Warning );
}
#endif
}
+// Informs the thread to quit
+void QgsSocketMonitoringThread::setResponseFinished( bool responseFinished )
+{
+ mIsResponseFinished.store( responseFinished );
+}
+
void QgsSocketMonitoringThread::run( )
{
+ const pid_t threadId = gettid();
+
if ( mIpcFd < 0 )
{
QgsMessageLog::logMessage( QStringLiteral( "Socket monitoring disabled: no socket fd!" ),
@@ -98,33 +115,55 @@ void QgsSocketMonitoringThread::run( )
}
#if defined(Q_OS_UNIX) && !defined(Q_OS_ANDROID)
+ mIsResponseFinished.store( false );
char c;
- while ( !*mIsResponseFinished )
+ while ( !mIsResponseFinished.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 );
- mFeedback->cancel();
- break;
+ // double check...
+ ssize_t x2 = 0;
+ for ( int i = 0; !mIsResponseFinished.load() && x2 == 0 && i < 10; i++ )
+ {
+ x2 = recv( mIpcFd, &c, 1, MSG_PEEK | MSG_DONTWAIT ); // see https://stackoverflow.com/a/12402596
+ std::this_thread::sleep_for( 50ms );
+ }
+ if ( x2 == 0 )
+ {
+ // socket closed, nothing can be read
+ QgsDebugMsgLevel( QStringLiteral( "FCGIServer %1: remote socket has been closed! errno: %2, x: %3" )
+ .arg( threadId ).arg( errno ).arg( x2 ), 2 );
+ mFeedback->cancel();
+ break;
+ }
+ else
+ {
+ QgsDebugMsgLevel( QStringLiteral( "FCGIServer::run %1: remote socket is not closed in fact! errno: %2, x: %3" )
+ .arg( threadId ).arg( errno ).arg( x2 ), 1 );
+ }
+
}
- 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 ( gSocketMonitoringMutex.try_lock_for( 333ms ) )
+ gSocketMonitoringMutex.unlock();
}
- if ( *mIsResponseFinished )
+ if ( mIsResponseFinished.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 ), 2 );
}
#endif
}
@@ -141,15 +180,28 @@ QgsFcgiServerResponse::QgsFcgiServerResponse( QgsServerRequest::Method method )
mBuffer.open( QIODevice::ReadWrite );
setDefaultHeaders();
- mSocketMonitoringThread = std::make_unique<QgsSocketMonitoringThread>( &mFinished, mFeedback.get() );
- mSocketMonitoringThread->start();
+ mSocketMonitoringThread = std::make_unique<QgsSocketMonitoringThread>( mFeedback );
+
+ // Lock the thread mutex: every try_lock will take 333ms
+ gSocketMonitoringMutex.lock();
+
+ // 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->setResponseFinished( mFinished );
+
+ // 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 'setResponseFinished' to true
+ gSocketMonitoringMutex.unlock();
+
+ // 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 25e6025d3bf3..a84ba11b8b67 100644
--- a/src/server/qgsfcgiserverresponse.h
+++ b/src/server/qgsfcgiserverresponse.h
@@ -26,7 +26,7 @@
#include "qgsserverresponse.h"
#include <QBuffer>
-#include <QThread>
+#include <thread>
/**
* \ingroup server
@@ -34,23 +34,30 @@
* \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<QgsFeedback> feedback );
+
+ /**
+ * main thread function
*/
- QgsSocketMonitoringThread( bool *isResponseFinished, QgsFeedback *feedback );
- void run( );
+ void run();
+
+ /**
+ * Controls thread life
+ * \param responseFinished stop the thread if true
+ */
+ void setResponseFinished( bool responseFinished );
private:
- bool *mIsResponseFinished = nullptr;
- QgsFeedback *mFeedback = nullptr;
+ std::atomic_bool mIsResponseFinished;
+ std::shared_ptr<QgsFeedback> mFeedback;
int mIpcFd = -1;
};
@@ -68,7 +75,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;
@@ -117,8 +125,12 @@ class SERVER_EXPORT QgsFcgiServerResponse: public QgsServerResponse
QgsServerRequest::Method mMethod;
int mStatusCode = 0;
+ // encapsulate thread data
std::unique_ptr<QgsSocketMonitoringThread> mSocketMonitoringThread;
- std::unique_ptr<QgsFeedback> mFeedback;
+ // real thread object. Used to join.
+ std::thread mThread;
+ // Used to cancel rendering jobs
+ std::shared_ptr<QgsFeedback> mFeedback;
};
#endif