Fix crash when transform errors occur while rendering

If a transform exception occurred while rendering a symbol then the QgsSymbolRenderContext cleanup code was never called, leading to a double delete and crash. Fixes #16377, #15345, and numerous other crashes seen "in the wild" Possibly refs #16385
qgis · May 7, 2017 · fefa572 · fefa572
1 parent 660aaa5
commit fefa572
Showing 1 changed file with 29 additions and 3 deletions.
diff --git a/src/core/symbology-ng/qgssymbol.cpp b/src/core/symbology-ng/qgssymbol.cpp
@@ -643,6 +643,27 @@ bool QgsSymbol::hasDataDefinedProperties() const
   return false;
 }
 
+///@cond PRIVATE
+
+/**
+ * RAII class to pop scope from an expression context on destruction
+ */
+class ExpressionContextScopePopper
+{
+  public:
+
+    ExpressionContextScopePopper() = default;
+
+    ~ExpressionContextScopePopper()
+    {
+      if ( context )
+        context->popScope();
+    }
+
+    QgsExpressionContext *context = nullptr;
+};
+///@endcond PRIVATE
+
 void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &context, int layer, bool selected, bool drawVertexMarker, int currentVertexMarkerType, int currentVertexMarkerSize )
 {
   QgsGeometry geom = feature.geometry();
@@ -672,9 +693,17 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
   mSymbolRenderContext->setGeometryPartCount( segmentizedGeometry.geometry()->partCount() );
   mSymbolRenderContext->setGeometryPartNum( 1 );
 
+  ExpressionContextScopePopper scopePopper;
   if ( mSymbolRenderContext->expressionContextScope() )
   {
+    // this is somewhat nasty - by appending this scope here it's now owned
+    // by both mSymbolRenderContext AND context.expressionContext()
+    // the RAII scopePopper is required to make sure it always has ownership transferred back
+    // from context.expressionContext(), even if exceptions of other early exits occur in this
+    // function
     context.expressionContext().appendScope( mSymbolRenderContext->expressionContextScope() );
+    scopePopper.context = &context.expressionContext();
+
     QgsExpressionContextUtils::updateSymbolScope( this, mSymbolRenderContext->expressionContextScope() );
     mSymbolRenderContext->expressionContextScope()->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_PART_COUNT, mSymbolRenderContext->geometryPartCount(), true ) );
     mSymbolRenderContext->expressionContextScope()->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_PART_NUM, 1, true ) );
@@ -952,9 +981,6 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
       }
     }
   }
-
-  if ( mSymbolRenderContext->expressionContextScope() )
-    context.expressionContext().popScope();
 }
 
 QgsSymbolRenderContext *QgsSymbol::symbolRenderContext()