diff --git a/.ci/ogc/build.sh b/.ci/ogc/build.sh index 7bbfb6d43d24..d53796c053ae 100755 --- a/.ci/ogc/build.sh +++ b/.ci/ogc/build.sh @@ -2,7 +2,7 @@ set -e -mkdir /usr/src/qgis/build +mkdir -p /usr/src/qgis/build cd /usr/src/qgis/build || exit 1 export CCACHE_TEMPDIR=/tmp diff --git a/.github/workflows/ogc.yml b/.github/workflows/ogc.yml index bd6dd3f82a60..fcea84294d3e 100644 --- a/.github/workflows/ogc.yml +++ b/.github/workflows/ogc.yml @@ -81,8 +81,8 @@ jobs: - name: Install pyogctest run: | sudo apt-get update && sudo apt-get install python3-virtualenv virtualenv git - git clone https://github.com/pblottiere/pyogctest - cd pyogctest && git checkout 1.1.1 && cd - + git clone https://github.com/benoitdm-oslandia/pyogctest + cd pyogctest && git checkout fix/docker_ulimit && cd - virtualenv -p /usr/bin/python3 venv && source venv/bin/activate && pip install -e pyogctest/ - name: Run WMS 1.3.0 OGC tests diff --git a/scripts/cppcheck.sh b/scripts/cppcheck.sh index 6ee87f5d63bb..83ccbd9baa63 100755 --- a/scripts/cppcheck.sh +++ b/scripts/cppcheck.sh @@ -14,10 +14,12 @@ case $SCRIPT_DIR in ;; esac +SRC_DIR=${1:-../src} + LOG_FILE=/tmp/cppcheck_qgis.txt rm -f ${LOG_FILE} -echo "Checking ${SCRIPT_DIR}/../src ..." +echo "Checking ${SCRIPT_DIR}/${SRC_DIR} ..." # qgsgcptransformer.cpp causes an effective hang on newer cppcheck! @@ -51,7 +53,7 @@ cppcheck --library=qt.cfg --inline-suppr \ -DBUILTIN_UNREACHABLE="__builtin_unreachable();" \ -i src/analysis/georeferencing/qgsgcptransformer.cpp \ -j $(nproc) \ - ${SCRIPT_DIR}/../src \ + ${SCRIPT_DIR}/${SRC_DIR} \ >>${LOG_FILE} 2>&1 & PID=$! diff --git a/src/server/qgsfcgiserverresponse.cpp b/src/server/qgsfcgiserverresponse.cpp index 7a61e34ef50d..4d53f84bb011 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,41 +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." ), + 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::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!" ), @@ -98,33 +116,59 @@ 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 ); - mFeedback->cancel(); - break; + // double check... + ssize_t x2 = 0; + for ( int i = 0; !mShouldStop.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 ( 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 } @@ -141,15 +185,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 ) @@ -296,5 +346,5 @@ void QgsFcgiServerResponse::truncate() void QgsFcgiServerResponse::setDefaultHeaders() { - setHeader( QStringLiteral( "Server" ), QStringLiteral( " QGIS FCGI server - QGIS version %1" ).arg( Qgis::version() ) ); + mHeaders.insert( QStringLiteral( "Server" ), QStringLiteral( " QGIS FCGI server - QGIS version %1" ).arg( Qgis::version() ) ); } diff --git a/src/server/qgsfcgiserverresponse.h b/src/server/qgsfcgiserverresponse.h index 25e6025d3bf3..69dc5618bd34 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,24 +35,33 @@ * \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( bool *isResponseFinished, QgsFeedback *feedback ); - void run( ); + QgsSocketMonitoringThread( std::shared_ptr feedback ); + + /** + * main thread function + */ + 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; }; /** @@ -68,7 +78,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 +128,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