Skip to content

Commit 9ea7f1e

Browse files
troopa81nyalldawson
authored andcommitted
Correct crash when read from ogr file in thread (QgsProcessing for instance)
fixes #20581 (cherry picked from commit e948120)
1 parent a249e28 commit 9ea7f1e

File tree

3 files changed

+84
-1
lines changed

3 files changed

+84
-1
lines changed

src/providers/ogr/qgsogrconnpool.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ class QgsOgrConnPool : public QgsConnectionPool<QgsOgrConn *, QgsOgrConnPoolGrou
156156

157157
if ( it.value()->unref() )
158158
{
159-
delete it.value();
159+
it.value()->deleteLater();
160160
mGroups.erase( it );
161161
}
162162
mMutex.unlock();

tests/src/providers/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ TARGET_LINK_LIBRARIES(qgis_arcgisrestutilstest arcgisfeatureserverprovider_a)
7070

7171
ADD_QGIS_TEST(gdalprovidertest testqgsgdalprovider.cpp)
7272

73+
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread")
7374
ADD_QGIS_TEST(ogrprovidertest testqgsogrprovider.cpp)
7475

7576
ADD_QGIS_TEST(wmscapabilitiestest

tests/src/providers/testqgsogrprovider.cpp

+82
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class TestQgsOgrProvider : public QObject
4545
void cleanup() {}// will be called after every testfunction.
4646

4747
void setupProxy();
48+
void testThread();
4849

4950
private:
5051
QString mTestDataDir;
@@ -119,6 +120,87 @@ void TestQgsOgrProvider::setupProxy()
119120

120121
}
121122

123+
class ReadVectorLayer : public QThread
124+
{
125+
126+
public :
127+
ReadVectorLayer( const QString &filePath, QMutex &mutex, QWaitCondition &waitForVlCreation, QWaitCondition &waitForProcessEvents )
128+
: _filePath( filePath ), _mutex( mutex ), _waitForVlCreation( waitForVlCreation ), _waitForProcessEvents( waitForProcessEvents ) {}
129+
130+
void run() override
131+
{
132+
133+
QgsVectorLayer *vl2 = new QgsVectorLayer( _filePath, QStringLiteral( "thread_test" ), QLatin1Literal( "ogr" ) );
134+
135+
QgsFeature f;
136+
QVERIFY( vl2->getFeatures().nextFeature( f ) );
137+
138+
_mutex.lock();
139+
_waitForVlCreation.wakeAll();
140+
_mutex.unlock();
141+
142+
_mutex.lock();
143+
_waitForProcessEvents.wait( &_mutex );
144+
_mutex.unlock();
145+
146+
delete vl2;
147+
}
148+
149+
private:
150+
QString _filePath;
151+
QMutex &_mutex;
152+
QWaitCondition &_waitForVlCreation;
153+
QWaitCondition &_waitForProcessEvents;
154+
155+
};
156+
157+
void failOnWarning( QtMsgType type, const QMessageLogContext &context, const QString &msg )
158+
{
159+
Q_UNUSED( context );
160+
161+
switch ( type )
162+
{
163+
case QtWarningMsg:
164+
QFAIL( QString( "No Qt warning message expect : %1" ).arg( msg ).toUtf8() );
165+
default:;
166+
}
167+
}
168+
169+
void TestQgsOgrProvider::testThread()
170+
{
171+
// After reading a QgsVectorLayer (getFeatures) from another thread the QgsOgrConnPoolGroup starts
172+
// an expiration timer. The timer belongs to the main thread in order to listening the event
173+
// loop and is parented to its QgsOgrConnPoolGroup. So when we delete the QgsVectorLayer, the
174+
// QgsConnPoolGroup and the timer are subsequently deleted from another thread. This leads to
175+
// segfault later when the expiration time reaches its timeout.
176+
177+
QMutex mutex;
178+
QWaitCondition waitForVlCreation;
179+
QWaitCondition waitForProcessEvents;
180+
181+
QString filePath = mTestDataDir + '/' + QStringLiteral( "lines.shp" );
182+
QThread *thread = new ReadVectorLayer( filePath, mutex, waitForVlCreation, waitForProcessEvents );
183+
184+
thread->start();
185+
186+
mutex.lock();
187+
waitForVlCreation.wait( &mutex );
188+
mutex.unlock();
189+
190+
// make sure timer as been started
191+
QCoreApplication::processEvents();
192+
193+
qInstallMessageHandler( failOnWarning );
194+
195+
mutex.lock();
196+
waitForProcessEvents.wakeAll();
197+
mutex.unlock();
198+
199+
thread->wait();
200+
qInstallMessageHandler( 0 );
201+
202+
}
203+
122204

123205
QGSTEST_MAIN( TestQgsOgrProvider )
124206
#include "testqgsogrprovider.moc"

0 commit comments

Comments
 (0)