vtkAbstractTransform::MyInverse is mistaken for a circular reference and incorrectly deleted under certain conditions
The simplest crash scenario is the following:
void testVtkCircularReferenceBug()
{
vtkSmartPointer<vtkTransform> trans = vtkSmartPointer<vtkTransform>::New();
// create sub-scope
{
vtkSmartPointer<vtkTransform> ref = trans;
// create sub-scope
{
vtkSmartPointer<vtkTransform> inv = vtkSmartPointer<vtkTransform>::New();
// the following line sets trans->MyInverse to inv and
// trans->DependsOnInverse to true
ref->SetInverse(inv);
}
}
// trans->MyInverse is incorrectly deleted and set to NULL when ref goes out
// of scope, leaving trans->DependsOnInverse as true
// the following line tries to access trans->MyInverse if
// trans->DependsOnInverse is true
trans->Update();
}
The problem seems to be in vtkAbstractTransform::UnRegister()
:
// check to see if the only reason our reference count is not 1
// is the circular reference from MyInverse
if (this->MyInverse && this->ReferenceCount == 2 &&
this->MyInverse->ReferenceCount == 1)
{ // break the cycle
vtkDebugMacro(<<"UnRegister: eliminating circular reference");
this->InUnRegister = 1;
this->MyInverse->UnRegister(this);
this->MyInverse = NULL;
this->InUnRegister = 0;
}
The above if-statement does not actually check whether this->MyInverse
actually contains a circular reference. The only place circular references are created is in vtkAbstractTransform::GetInverse()
. The above if-condition should be changed to the following:
if (this->MyInverse && this->MyInverse->MyInverse == this &&
this->ReferenceCount == 2 &&
this->MyInverse->ReferenceCount == 1)
I'm am still waiting for a build to verify that the above fix works. Is my analysis of the issue correct?
We are using VTK 7.1.1, but it looks like this bug has been around since VTK 4 and is still present in VTK 8 and the latest master branch.