Skip to content

Commit 9df1a08

Browse files
blazekblazek
committedSep 26, 2015
[GRASS] fixed race condition in reading import progress
·
ltr-3_40archive/master_2
1 parent e6e79c1 commit 9df1a08

File tree

7 files changed

+222
-83
lines changed

7 files changed

+222
-83
lines changed
 

‎src/plugins/grass/qgsgrassmodule.cpp

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -897,34 +897,16 @@ void QgsGrassModule::readStderr()
897897
line = QString::fromLocal8Bit( ba ).replace( '\n', "" );
898898
//QgsDebugMsg(QString("line: '%1'").arg(line));
899899

900-
if ( rxpercent.indexIn( line ) != -1 )
901-
{
902-
int progress = rxpercent.cap( 1 ).toInt();
903-
mProgressBar->setValue( progress );
904-
}
905-
else if ( rxmessage.indexIn( line ) != -1 )
906-
{
907-
mOutputTextBrowser->append( "<pre>" + rxmessage.cap( 1 ) + "</pre>" );
908-
}
909-
else if ( rxwarning.indexIn( line ) != -1 )
900+
QString text, html;
901+
int percent;
902+
QgsGrass::ModuleOutput type = QgsGrass::parseModuleOutput( line, text, html, percent );
903+
if ( type == QgsGrass::OutputPercent )
910904
{
911-
QString warn = rxwarning.cap( 1 );
912-
QString img = QgsApplication::pkgDataPath() + "/themes/default/grass/grass_module_warning.png";
913-
mOutputTextBrowser->append( "<img src=\"" + img + "\">" + warn );
905+
mProgressBar->setValue( percent );
914906
}
915-
else if ( rxerror.indexIn( line ) != -1 )
907+
else if ( type == QgsGrass::OutputMessage || type == QgsGrass::OutputWarning || type == QgsGrass::OutputError )
916908
{
917-
QString error = rxerror.cap( 1 );
918-
QString img = QgsApplication::pkgDataPath() + "/themes/default/grass/grass_module_error.png";
919-
mOutputTextBrowser->append( "<img src=\"" + img + "\">" + error );
920-
}
921-
else if ( rxend.indexIn( line ) != -1 )
922-
{
923-
// Do nothing
924-
}
925-
else
926-
{
927-
mOutputTextBrowser->append( "<pre>" + line + "</pre>" );
909+
mOutputTextBrowser->append( html );
928910
}
929911
}
930912
}

‎src/providers/grass/qgsgrass.cpp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1752,6 +1752,7 @@ QProcess *QgsGrass::startModule( const QString& gisdbase, const QString& locati
17521752
gisrcFile.close();
17531753
QStringList environment = QProcess::systemEnvironment();
17541754
environment.append( "GISRC=" + gisrcFile.fileName() );
1755+
environment.append( "GRASS_MESSAGE_FORMAT=gui" );
17551756

17561757
process->setEnvironment( environment );
17571758

@@ -2646,3 +2647,53 @@ void QgsGrass::sleep( int ms )
26462647
nanosleep( &ts, NULL );
26472648
#endif
26482649
}
2650+
2651+
QgsGrass::ModuleOutput QgsGrass::parseModuleOutput( const QString & input, QString &text, QString &html, int &percent )
2652+
{
2653+
QRegExp rxpercent( "GRASS_INFO_PERCENT: (\\d+)" );
2654+
QRegExp rxmessage( "GRASS_INFO_MESSAGE\\(\\d+,\\d+\\): (.*)" );
2655+
QRegExp rxwarning( "GRASS_INFO_WARNING\\(\\d+,\\d+\\): (.*)" );
2656+
QRegExp rxerror( "GRASS_INFO_ERROR\\(\\d+,\\d+\\): (.*)" );
2657+
QRegExp rxend( "GRASS_INFO_END\\(\\d+,\\d+\\)" );
2658+
2659+
2660+
if ( input.trimmed().isEmpty() )
2661+
{
2662+
return OutputNone;
2663+
}
2664+
else if ( rxpercent.indexIn( input ) != -1 )
2665+
{
2666+
percent = rxpercent.cap( 1 ).toInt();
2667+
return OutputPercent;
2668+
}
2669+
else if ( rxmessage.indexIn( input ) != -1 )
2670+
{
2671+
text = rxmessage.cap( 1 );
2672+
html = "<pre>" + text + "</pre>" ;
2673+
return OutputMessage;
2674+
}
2675+
else if ( rxwarning.indexIn( input ) != -1 )
2676+
{
2677+
text = rxwarning.cap( 1 );
2678+
QString img = QgsApplication::pkgDataPath() + "/themes/default/grass/grass_module_warning.png";
2679+
html = "<img src=\"" + img + "\">" + text;
2680+
return OutputWarning;
2681+
}
2682+
else if ( rxerror.indexIn( input ) != -1 )
2683+
{
2684+
text = rxerror.cap( 1 );
2685+
QString img = QgsApplication::pkgDataPath() + "/themes/default/grass/grass_module_error.png";
2686+
html = "<img src=\"" + img + "\">" + text;
2687+
return OutputError;
2688+
}
2689+
else if ( rxend.indexIn( input ) != -1 )
2690+
{
2691+
return OutputNone;
2692+
}
2693+
else // some plain text which cannot be parsed
2694+
{
2695+
text = input;
2696+
html = "<pre>" + text + "</pre>";
2697+
return OutputMessage;
2698+
}
2699+
}

‎src/providers/grass/qgsgrass.h

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,16 @@ class GRASS_LIB_EXPORT QgsGrass : public QObject
140140
public:
141141
static jmp_buf jumper; // used to get back from fatal error
142142

143+
// Parsed module output
144+
enum ModuleOutput
145+
{
146+
OutputNone,
147+
OutputPercent,
148+
OutputMessage,
149+
OutputWarning,
150+
OutputError
151+
};
152+
143153
// This does not work (gcc/Linux), such exception cannot be caught
144154
// so I have enabled the old version, if you are able to fix it, please
145155
// check first if it realy works, i.e. can be caught!
@@ -558,11 +568,18 @@ class GRASS_LIB_EXPORT QgsGrass : public QObject
558568
// Free struct Map_info
559569
static void vectDestroyMapStruct( struct Map_info *map );
560570

561-
// Sleep miliseconds (for debugging)
571+
// Sleep miliseconds (for debugging), does not work on threads(?)
562572
static void sleep( int ms );
563573

564574
void emitNewLayer( QString uri, QString name ) { emit newLayer( uri, name ); }
565575

576+
/** Parse single line of output from GRASS modules run with GRASS_MESSAGE_FORMAT=gui
577+
* @param input input string read from module stderr
578+
* @param text parsed text
579+
* @param html html formated parsed text, e.g. + icons
580+
* @param percent progress 0-100 */
581+
static ModuleOutput parseModuleOutput( const QString & input, QString &text, QString &html, int &percent );
582+
566583
public slots:
567584
/** Close mapset and show warning if closing failed */
568585
bool closeMapsetWarn();

‎src/providers/grass/qgsgrassimport.cpp

Lines changed: 84 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,81 @@ QgsGrassImportIcon::QgsGrassImportIcon()
4848
{
4949
}
5050

51+
//------------------------------ QgsGrassImportProcess ------------------------------------
52+
QgsGrassImportProgress::QgsGrassImportProgress( QProcess *process, QObject *parent )
53+
: QObject( parent )
54+
, mProcess( process )
55+
, mProgressMin( 0 )
56+
, mProgressMax( 0 )
57+
, mProgressValue( 0 )
58+
{
59+
connect( mProcess, SIGNAL( readyReadStandardError() ), SLOT( onReadyReadStandardError() ) );
60+
}
61+
62+
void QgsGrassImportProgress::setProcess( QProcess *process )
63+
{
64+
mProcess = process;
65+
connect( mProcess, SIGNAL( readyReadStandardError() ), SLOT( onReadyReadStandardError() ) );
66+
}
67+
68+
void QgsGrassImportProgress::onReadyReadStandardError()
69+
{
70+
QgsDebugMsg( "entered" );
71+
if ( mProcess )
72+
{
73+
// TODO: parse better progress output
74+
QString output = QString::fromLocal8Bit( mProcess->readAllStandardError() );
75+
Q_FOREACH ( QString line, output.split( "\n" ) )
76+
{
77+
QgsDebugMsg( "line = '" + line + "'" );
78+
QString text, html;
79+
int percent;
80+
QgsGrass::ModuleOutput type = QgsGrass::parseModuleOutput( line, text, html, percent );
81+
if ( type == QgsGrass::OutputPercent )
82+
{
83+
mProgressMin = 0;
84+
mProgressMax = 100;
85+
mProgressValue = percent;
86+
emit progressChanged( html, mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
87+
}
88+
else if ( type == QgsGrass::OutputMessage || type == QgsGrass::OutputWarning || type == QgsGrass::OutputError )
89+
{
90+
mProgressHtml += html;
91+
QgsDebugMsg( "text = " + text );
92+
emit progressChanged( html, mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
93+
}
94+
}
95+
}
96+
}
97+
98+
void QgsGrassImportProgress::append( const QString & html )
99+
{
100+
mProgressHtml += html;
101+
emit progressChanged( html, mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
102+
}
103+
104+
void QgsGrassImportProgress::setRange( int min, int max )
105+
{
106+
mProgressMin = min;
107+
mProgressMax = max;
108+
mProgressValue = min;
109+
emit progressChanged( "", mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
110+
}
111+
112+
void QgsGrassImportProgress::setValue( int value )
113+
{
114+
mProgressValue = value;
115+
emit progressChanged( "", mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
116+
}
117+
51118
//------------------------------ QgsGrassImport ------------------------------------
52119
QgsGrassImport::QgsGrassImport( QgsGrassObject grassObject )
53120
: QObject()
54121
, mGrassObject( grassObject )
55122
, mCanceled( false )
56123
, mProcess( 0 )
124+
, mProgress( 0 )
57125
, mFutureWatcher( 0 )
58-
, mProgressMin( 0 )
59-
, mProgressMax( 0 )
60-
, mProgressValue( 0 )
61126
{
62127
// QMovie used by QgsAnimatedIcon is using QTimer which cannot be start from another thread
63128
// (it works on Linux however) so we cannot start it connecting from QgsGrassImportItem and
@@ -125,28 +190,6 @@ void QgsGrassImport::cancel()
125190
mCanceled = true;
126191
}
127192

128-
void QgsGrassImport::emitProgressChanged()
129-
{
130-
emit progressChanged( mProgressHtml + mProgressTmpHtml, mProgressMin, mProgressMax, mProgressValue );
131-
132-
}
133-
134-
void QgsGrassImport::onReadyReadStandardError()
135-
{
136-
if ( mProcess )
137-
{
138-
// TODO: should be locked? Lock does not help anyway.
139-
// TODO: parse better progress output
140-
mProgressHtml += QString( mProcess->readAllStandardError() ).replace( "\n", "<br>" );
141-
emitProgressChanged();
142-
}
143-
}
144-
145-
void QgsGrassImport::addProgressRow( QString html )
146-
{
147-
mProgressHtml += html + "<br>";
148-
}
149-
150193
//------------------------------ QgsGrassRasterImport ------------------------------------
151194
QgsGrassRasterImport::QgsGrassRasterImport( QgsRasterPipe* pipe, const QgsGrassObject& grassObject,
152195
const QgsRectangle &extent, int xSize, int ySize )
@@ -196,8 +239,6 @@ bool QgsGrassRasterImport::import()
196239
for ( int band = 1; band <= provider->bandCount(); band++ )
197240
{
198241
QgsDebugMsg( QString( "band = %1" ).arg( band ) );
199-
addProgressRow( tr( "Writing band %1/%2" ).arg( band ).arg( provider->bandCount() ) );
200-
emitProgressChanged();
201242
int colorInterpretation = provider->colorInterpretation( band );
202243
if ( colorInterpretation == QgsRaster::RedBand )
203244
{
@@ -265,6 +306,15 @@ bool QgsGrassRasterImport::import()
265306
setError( e.what() );
266307
return false;
267308
}
309+
if ( !mProgress )
310+
{
311+
mProgress = new QgsGrassImportProgress( mProcess, this );
312+
}
313+
else
314+
{
315+
mProgress->setProcess( mProcess );
316+
}
317+
mProgress->append( tr( "Writing band %1/%2" ).arg( band ).arg( provider->bandCount() ) );
268318

269319
QDataStream outStream( mProcess );
270320

@@ -292,13 +342,13 @@ bool QgsGrassRasterImport::import()
292342
int iterRows = 0;
293343
QgsRasterBlock* block = 0;
294344
mProcess->setReadChannel( QProcess::StandardOutput );
295-
mProgressMax = mYSize;
345+
mProgress->setRange( 0, mYSize - 1 );
296346
while ( iter.readNextRasterPart( band, iterCols, iterRows, &block, iterLeft, iterTop ) )
297347
{
298348
for ( int row = 0; row < iterRows; row++ )
299349
{
300-
mProgressValue = iterTop + row;
301-
emitProgressChanged();
350+
mProgress->setValue( iterTop + row );
351+
302352
if ( !block->convert( qgis_out_type ) )
303353
{
304354
setError( tr( "Cannot convert block (%1) to data type %2" ).arg( block->toString() ).arg( qgis_out_type ) );
@@ -385,7 +435,6 @@ bool QgsGrassRasterImport::import()
385435
if ( mProcess->exitStatus() != QProcess::NormalExit )
386436
{
387437
setError( mProcess->errorString() );
388-
delete mProcess;
389438
mProcess = 0;
390439
return false;
391440
}
@@ -528,8 +577,7 @@ bool QgsGrassVectorImport::import()
528577
setError( e.what() );
529578
return false;
530579
}
531-
// TODO: connecting readyReadStandardError() is causing hangs or crashes
532-
//connect(mProcess, SIGNAL(readyReadStandardError()), this, SLOT(onReadyReadStandardError()));
580+
mProgress = new QgsGrassImportProgress( mProcess, this );
533581

534582
QDataStream outStream( mProcess );
535583
mProcess->setReadChannel( QProcess::StandardOutput );
@@ -542,10 +590,10 @@ bool QgsGrassVectorImport::import()
542590

543591
QgsFeatureIterator iterator = mProvider->getFeatures();
544592
QgsFeature feature;
545-
mProgressMax = mProvider->featureCount();
593+
mProgress->setRange( 1, mProvider->featureCount() );
594+
mProgress->append( tr( "Writing features" ) );
546595
for ( int i = 0; i < ( isPolygon ? 2 : 1 ); i++ ) // two cycles with polygons
547596
{
548-
addProgressRow( tr( "Writing features" ) );
549597
if ( i > 0 ) // second run for polygons
550598
{
551599
//iterator.rewind(); // rewind does not work
@@ -555,9 +603,7 @@ bool QgsGrassVectorImport::import()
555603
int count = 0;
556604
while ( iterator.nextFeature( feature ) )
557605
{
558-
mProgressTmpHtml = tr( "Feature %1/%2" ).arg( count + 1 ).arg( mProgressMax );
559-
mProgressValue = count + 1;
560-
emitProgressChanged();
606+
mProgress->setValue( count + 1 );
561607
if ( !feature.isValid() )
562608
{
563609
continue;
@@ -603,7 +649,6 @@ bool QgsGrassVectorImport::import()
603649
outStream >> result;
604650
#endif
605651
}
606-
607652
iterator.close();
608653

609654
// Close write channel before waiting for response to avoid stdin buffer problem on Windows

‎src/providers/grass/qgsgrassimport.h

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,40 @@ class GRASS_LIB_EXPORT QgsGrassImportIcon : public QgsAnimatedIcon
3535
virtual ~QgsGrassImportIcon() {};
3636
};
3737

38+
// QgsGrassImport items live on the main thread but mProcess, when importInThread() is used, lives on another
39+
// thread. When QProcess::readyReadStandardError() is connected to QgsGrassImport, it is processed
40+
// correctly on the main thread, but we cannot use there mProcess->readAllStandardError() because the process
41+
// is running on another thread. That is why we create QgsGrassImportProgress on the same thread as the process.
42+
class GRASS_LIB_EXPORT QgsGrassImportProgress : public QObject
43+
{
44+
Q_OBJECT
45+
public:
46+
QgsGrassImportProgress( QProcess *process, QObject *parent = 0 );
47+
48+
void setProcess( QProcess *process );
49+
QString progressHtml() { return mProgressHtml; }
50+
51+
void append( const QString & html );
52+
void setRange( int min, int max );
53+
void setValue( int value );
54+
55+
public slots:
56+
void onReadyReadStandardError();
57+
58+
signals:
59+
void progressChanged( const QString &recentHtml, const QString &allHtml, int min, int max, int value );
60+
61+
private:
62+
QProcess* mProcess;
63+
// All stderr read from the modules converted to HTML
64+
QString mProgressHtml;
65+
// temporary part of progress, e.g. number of features written.
66+
QString mProgressTmpHtml;
67+
int mProgressMin;
68+
int mProgressMax;
69+
int mProgressValue;
70+
};
71+
3872
class GRASS_LIB_EXPORT QgsGrassImport : public QObject
3973
{
4074
Q_OBJECT
@@ -50,7 +84,7 @@ class GRASS_LIB_EXPORT QgsGrassImport : public QObject
5084
QString error();
5185
virtual QStringList names() const;
5286
bool isCanceled() const;
53-
void emitProgressChanged();
87+
QgsGrassImportProgress * progress() { return mProgress; }
5488
public slots:
5589
void onFinished();
5690
// TODO: this is not completely kosher, because QgsGrassImport exist on the main thread
@@ -61,14 +95,10 @@ class GRASS_LIB_EXPORT QgsGrassImport : public QObject
6195
void cancel();
6296
void frameChanged() {}
6397

64-
void onReadyReadStandardError();
65-
6698
signals:
6799
// sent when process finished
68100
void finished( QgsGrassImport *import );
69101

70-
void progressChanged( QString html, int min, int max, int value );
71-
72102
protected:
73103
static bool run( QgsGrassImport *imp );
74104
void setError( QString error );
@@ -77,13 +107,8 @@ class GRASS_LIB_EXPORT QgsGrassImport : public QObject
77107
QString mError;
78108
bool mCanceled;
79109
QProcess* mProcess;
110+
QgsGrassImportProgress* mProgress;
80111
QFutureWatcher<bool>* mFutureWatcher;
81-
QString mProgressHtml;
82-
// temporary part of progress, e.g. number of features written.
83-
QString mProgressTmpHtml;
84-
int mProgressMin;
85-
int mProgressMax;
86-
int mProgressValue;
87112
};
88113

89114
class GRASS_LIB_EXPORT QgsGrassRasterImport : public QgsGrassImport

‎src/providers/grass/qgsgrassprovidermodule.cpp

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,15 +1088,27 @@ QgsGrassImportItemWidget::QgsGrassImportItemWidget( QWidget* parent )
10881088
layout->addWidget( mProgressBar );
10891089
}
10901090

1091-
void QgsGrassImportItemWidget::onProgressChanged( QString html, int min, int max, int value )
1091+
void QgsGrassImportItemWidget::setHtml( const QString & html )
10921092
{
1093-
mTextEdit->setHtml( html );
1093+
if ( mTextEdit )
1094+
{
1095+
mTextEdit->setText( html );
1096+
}
1097+
}
1098+
1099+
void QgsGrassImportItemWidget::onProgressChanged( const QString &recentHtml, const QString &allHtml, int min, int max, int value )
1100+
{
1101+
Q_UNUSED( allHtml );
1102+
if ( !recentHtml.isEmpty() )
1103+
{
1104+
mTextEdit->append( recentHtml );
1105+
}
1106+
// TODO: scroll to bottom
10941107
mTextEdit->verticalScrollBar()->setValue( mTextEdit->verticalScrollBar()->maximum() );
10951108
mProgressBar->setRange( min, max );
10961109
mProgressBar->setValue( value );
10971110
}
10981111

1099-
11001112
//----------------------- QgsGrassImportItem ------------------------------
11011113

11021114
QgsAnimatedIcon *QgsGrassImportItem::mImportIcon = 0;
@@ -1132,9 +1144,14 @@ QWidget * QgsGrassImportItem::paramWidget()
11321144
{
11331145
QgsDebugMsg( "entered" );
11341146
QgsGrassImportItemWidget *widget = new QgsGrassImportItemWidget();
1135-
connect( mImport, SIGNAL( progressChanged( QString, int, int, int ) ),
1136-
widget, SLOT( onProgressChanged( QString, int, int, int ) ) );
1137-
mImport->emitProgressChanged();
1147+
1148+
if ( mImport && mImport->progress() )
1149+
{
1150+
connect( mImport->progress(), SIGNAL( progressChanged( const QString &, const QString &, int, int, int ) ),
1151+
widget, SLOT( onProgressChanged( const QString &, const QString &, int, int, int ) ) );
1152+
1153+
widget->setHtml( mImport->progress()->progressHtml() );
1154+
}
11381155
return widget;
11391156
}
11401157

‎src/providers/grass/qgsgrassprovidermodule.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,10 @@ class QgsGrassImportItemWidget : public QWidget
195195
public:
196196
QgsGrassImportItemWidget( QWidget* parent = 0 );
197197

198+
void setHtml( const QString & html );
199+
198200
public slots:
199-
void onProgressChanged( QString html, int min, int max, int value );
201+
void onProgressChanged( const QString &recentHtml, const QString &allHtml, int min, int max, int value );
200202

201203
private:
202204
QTextEdit *mTextEdit;

0 commit comments

Comments
 (0)
Please sign in to comment.