Iterator debugging crashes when reassigning an stl iterator in an object destructor within a CArray - by Dave Wilcoxson

Status : 

  By Design<br /><br />
		The product team believes this item works according to its intended design.<br /><br />
		A more detailed explanation for the resolution of this particular item may have been provided in the comments section.


1
0
Sign in
to vote
ID 778467 Comments
Status Closed Workarounds
Type Bug Repros 0
Opened 2/4/2013 8:11:23 AM
Access Restriction Public

Description

The following very trivial code will cause a crash when using iterator debugging level 2 (the default) in VS2012.  I stripped this down from a much larger code.

#include <set>

using namespace std;

class Point
{
  set<int> m_rgX;
  set<int> m_rgY;

public:

  ~Point()
  {
    set<int>::iterator it = m_rgX.begin();
    it = m_rgY.begin();
  }
};

int _tmain(int argc, TCHAR* argv[], TCHAR* envp[])
{
  CArray<Point> rgPoints;
  rgPoints.Add(Point());
  rgPoints.Add(Point());

  return 0;
}
Sign in to post a comment.
Posted by Microsoft on 2/6/2013 at 8:22 PM
Hi,

Thanks for reporting this issue. I've resolved it as By Design because CArray was designed for Plain Old Data only, and STL containers like std::set are inherently non-POD.

If you set a breakpoint on "CArray<Point,Point const &>::SetSize" without quotes, you can see what's happening. This invokes:

// copy new data from old
::ATL::Checked::memcpy_s(pNewData, (size_t)nNewMax * sizeof(TYPE),
    m_pData, (size_t)m_nSize * sizeof(TYPE));

That is, when a CArray expands, it calls memcpy to blast bits from the old memory block to the new memory block, instead of invoking copy constructors and destructors. This is fine for Plain Old Data (ints, floats, structs of ints, etc.) but is completely wrong for objects with copy constructors and destructors, like std::set. (Physically, what's happening is that in debug mode, STL containers keep track of all of their iterators, in order to emit extremely useful assertions when invalidated iterators are used and so forth. This is done by storing extra data members in containers and iterators, and updating those data members when things are copied/destroyed/etc. Because memcpy bypasses copy constructors, the bookkeeping information for iterator debugging is totally damaged, leading to a crash.)

This is actually documented at http://msdn.microsoft.com/en-us/library/4h2f09ct.aspx : "Most methods that resize a CArray object or add elements to it use memcpy_s to move elements. This is a problem because memcpy_s is not compatible with any objects that require the constructor to be called. If the items in the CArray are not compatible with memcpy_s, you must create a new CArray of the appropriate size. You must then use CArray::Copy and CArray::SetAt to populate the new array because those methods use an assignment operator instead of memcpy_s."

My recommendation goes beyond MSDN's: avoid using CArray, period. Instead, use std::vector (which respects copy constructors, and is highly optimized - it'll invoke C++11 move constructors when possible, and when it senses int/etc. elements it'll invoke memmove to blast bits). If you must use CArray (e.g. when talking to other ATL/MFC code), make sure to use it with PODs only.

Note: Connect doesn't notify me about comments. If you have any further questions, please E-mail me.

Stephan T. Lavavej
Senior Developer - Visual C++ Libraries
stl@microsoft.com
Posted by Microsoft on 2/4/2013 at 11:02 PM
Thanks for your feedback.

We are rerouting this issue to the appropriate group within the Visual Studio Product Team for triage and resolution. These specialized experts will follow-up with your issue.
Posted by Microsoft on 2/4/2013 at 8:50 AM
Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)