Skip to content

Commit

Permalink
Fix crash when transform errors occur while rendering
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nyalldawson committed May 7, 2017
1 parent 660aaa5 commit fefa572
Showing 1 changed file with 29 additions and 3 deletions.
32 changes: 29 additions & 3 deletions src/core/symbology-ng/qgssymbol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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 ) );
Expand Down Expand Up @@ -952,9 +981,6 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
}
}
}

if ( mSymbolRenderContext->expressionContextScope() )
context.expressionContext().popScope();
}

QgsSymbolRenderContext *QgsSymbol::symbolRenderContext()
Expand Down

0 comments on commit fefa572

Please sign in to comment.