Skip to content
Permalink
Browse files

Prevents a crash when online features are gone

Fixes an unreported segfault when converting an online layer
to offline if features cannot be (completely) fetched.

Funded by Boundless
  • Loading branch information
elpaso committed Apr 15, 2016
1 parent eeb9bdd commit 79442b6ccb29602d3e34db6a42075075321f92fd
Showing with 13 additions and 5 deletions.
  1. +13 −5 src/core/qgsofflineediting.cpp
@@ -129,6 +129,9 @@ bool QgsOfflineEditing::convertToOfflineProject( const QString& offlineDataPath,
if ( newLayer )
{
layerIdMapping.insert( origLayerId, newLayer );
// remove remote layer
QgsMapLayerRegistry::instance()->removeMapLayers(
QStringList() << origLayerId );
}
}

@@ -645,7 +648,16 @@ QgsVectorLayer* QgsOfflineEditing::copyVectorLayer( QgsVectorLayer* layer, sqlit
int remoteCount = remoteFeatureIds.size();
for ( int i = 0; i < remoteCount; i++ )
{
addFidLookup( db, layerId, offlineFeatureIds.at( i ), remoteFeatureIds.at( remoteCount - ( i + 1 ) ) );
// Check if the online feature has been fetched (WFS download aborted for some reason)
if ( i < offlineFeatureIds.count() )
{
addFidLookup( db, layerId, offlineFeatureIds.at( i ), remoteFeatureIds.at( remoteCount - ( i + 1 ) ) );
}
else
{
showWarning( QString( "Feature cannot be copied to the offline layer, please check if the online layer '%1' is sill accessible." ).arg( layer->name() ) );
return nullptr;

This comment has been minimized.

Copy link
@m-kuhn

m-kuhn Jun 22, 2016

Member

@elpaso
shouldn't the transaction be rolled back here?
Sidenote; I normally prefer not to return in the middle of a function to avoid having skipped cleanup steps at the end of a function.

This comment has been minimized.

Copy link
@elpaso

elpaso Jun 22, 2016

Author Contributor

If you rollback you get 0 features, right?
I believe that in a real situation you just get 0 features or all of them from the remote endpoint, IIRC that's the case I faced when I discovered this issue.

This comment has been minimized.

Copy link
@m-kuhn

m-kuhn Jun 22, 2016

Member

Yes, the fidlookup will be empty with a rollback. I would have to check for what happens to all the other tables.

Not sure what's the correct way to deal with it, but either it should commit the available features or rollback. Leaving an unfinished transaction open is probably not the safest solution.

}
emit progressUpdated( featureCount++ );
}
sqlExec( db, "COMMIT" );
@@ -654,10 +666,6 @@ QgsVectorLayer* QgsOfflineEditing::copyVectorLayer( QgsVectorLayer* layer, sqlit
{
showWarning( newLayer->commitErrors().join( "\n" ) );
}

// remove remote layer
QgsMapLayerRegistry::instance()->removeMapLayers(
QStringList() << layer->id() );
}
return newLayer;
}

0 comments on commit 79442b6

Please sign in to comment.
You can’t perform that action at this time.