Using a code snippet like this
{
mitk::ImagePixelReadAccessor<mitk::ScalarType,3> accessor = mitk::ImagePixelReadAccessor<mitk::ScalarType,3>(image);
}
will cause an access violation as soon as we leave scope.
Reason:
The default copy operator is not explicitly deactivated, leading to the left side accessor to be constructed be mere copying. Thus no ImageReadAccessor:: OrganizeReadAccess() is called.
The Destructor of the right side accessor (RSA) clears m_Image->m_Readers.
Now we have an accessor that is not registered and an image with an empty readers vector.
If we leave the scope, the RSA destructor tries to remove the reader.
The next issue comes with line 74-75 (mitkImageReadAccessor.h)
74: std::vector<ImageAccessorBase*>::iterator it = std::find(m_Image->m_Readers.begin(),m_Image->m_Readers.end(),this);
75: m_Image->m_Readers.erase(it);
Now in line 75 erase is called with "it == m_Image->m_Readers.end()"
Calling std::vector::erase on the end-iterator is undefined behavior (which causes an access violation on windows platforms).
Proposals:
- Add check to ~ImageReadAccessor(), to avoid undefined behavior.
std::vector<ImageAccessorBase*>::iterator it = std::find(m_Image->m_Readers.begin(),m_Image->m_Readers.end(),this);
if (it != m_Image->m_Readers.end())
{
m_Image->m_Readers.erase(it);
}
- Explicitly remove operator=() to avoid this wrong usage by design
or implement it explicitly and correctly ;)