Skip to content

Commit 8a5bcf9

Browse files
committed
[wfs] Fix some race conditions
(cherry picked from commit 2041cad)
1 parent bf10aca commit 8a5bcf9

File tree

2 files changed

+26
-20
lines changed

2 files changed

+26
-20
lines changed

src/providers/wfs/qgswfsrequest.cpp

+23-11
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,15 @@ bool QgsWfsRequest::sendGET( const QUrl &url, bool synchronous, bool forceRefres
130130

131131
QWaitCondition waitCondition;
132132
QMutex waitConditionMutex;
133+
bool threadFinished = false;
134+
bool success = false;
133135

134-
std::function<bool()> downloaderFunction = [ this, request, synchronous, &waitConditionMutex, &waitCondition ]()
136+
std::function<void()> downloaderFunction = [ this, request, synchronous, &waitConditionMutex, &waitCondition, &threadFinished, &success ]()
135137
{
136138
if ( QThread::currentThread() != QgsApplication::instance()->thread() )
137139
QgsNetworkAccessManager::instance( Qt::DirectConnection );
138140

139-
bool success = true;
141+
success = true;
140142
mReply = QgsNetworkAccessManager::instance()->get( request );
141143

142144
mReply->setReadBufferSize( READ_BUFFER_SIZE_HINT );
@@ -180,41 +182,51 @@ bool QgsWfsRequest::sendGET( const QUrl &url, bool synchronous, bool forceRefres
180182
loop.exec();
181183
}
182184
}
185+
waitConditionMutex.lock();
186+
threadFinished = true;
183187
waitCondition.wakeAll();
184-
return success;
188+
waitConditionMutex.unlock();
185189
};
186190

187-
bool success;
188-
189191
if ( synchronous && QThread::currentThread() == QApplication::instance()->thread() )
190192
{
191193
std::unique_ptr<DownloaderThread> downloaderThread = qgis::make_unique<DownloaderThread>( downloaderFunction );
192194
downloaderThread->start();
193195

194-
while ( !downloaderThread->isFinished() )
196+
while ( true )
195197
{
196198
waitConditionMutex.lock();
199+
if ( threadFinished )
200+
{
201+
waitConditionMutex.unlock();
202+
break;
203+
}
197204
waitCondition.wait( &waitConditionMutex );
198-
waitConditionMutex.unlock();
199205

200206
// If the downloader thread wakes us (the main thread) up and is not yet finished
201207
// he needs the authentication to run.
202208
// The processEvents() call gives the auth manager the chance to show a dialog and
203209
// once done with that, we can wake the downloaderThread again and continue the download.
204-
if ( !downloaderThread->isFinished() )
210+
if ( !threadFinished )
205211
{
212+
waitConditionMutex.unlock();
213+
206214
QgsApplication::instance()->processEvents();
207215
waitConditionMutex.lock();
208216
waitCondition.wakeAll();
209217
waitConditionMutex.unlock();
210218
}
219+
else
220+
{
221+
waitConditionMutex.unlock();
222+
}
211223
}
212-
213-
success = downloaderThread->success();
224+
// wait for thread to gracefully exit
225+
downloaderThread->wait();
214226
}
215227
else
216228
{
217-
success = downloaderFunction();
229+
downloaderFunction();
218230
}
219231
return success && mErrorMessage.isEmpty();
220232
}

src/providers/wfs/qgswfsrequest.h

+3-9
Original file line numberDiff line numberDiff line change
@@ -124,25 +124,19 @@ class DownloaderThread : public QThread
124124
Q_OBJECT
125125

126126
public:
127-
DownloaderThread( std::function<bool()> function, QObject *parent = nullptr )
127+
DownloaderThread( std::function<void()> function, QObject *parent = nullptr )
128128
: QThread( parent )
129129
, mFunction( function )
130130
{
131131
}
132132

133133
void run() override
134134
{
135-
mSuccess = mFunction();
136-
}
137-
138-
bool success() const
139-
{
140-
return mSuccess;
135+
mFunction();
141136
}
142137

143138
private:
144-
std::function<bool()> mFunction;
145-
bool mSuccess = false;
139+
std::function<void()> mFunction;
146140
};
147141

148142
#endif // QGSWFSREQUEST_H

0 commit comments

Comments
 (0)