• entries
    13
  • comments
    15
  • views
    29117

Today's Lesson (for me): Pointers that mysteriously get deleted!

Sign in to follow this  
Followers 0
MilchoPenchev

746 views

I'm posting this as an excercise/lesson, hopefully its useful to someone.
Eight years after I started learning c++, I was still caught off guard by this.

Basically I had code like this: (ignore the LOG_DEBUG - that was just put there when I was testing this)struct ViewMember{ ViewMember(Widget *wid, int wei) : widget(wid), weight(wei) { } ~ViewMember() { LOG_DEBUG << "calling View Member destructor"; delete widget; } Widget *widget; int weight;};now, in a class called View, I have a member - widgetList is of type std::vectorView& View::AddWidget (Widget *toAdd, int weight){ if (weight < 1) weight = 1; if (Contains(toAdd)) return (*this); // won't add same widget again widgetList.push_back(ViewMember(toAdd, weight)); needToReorganize = true; return (*this);} This code (barring typos I may have made in copy/re-arrange) compiles fine, without warnings or errors.

However, once I actually tried to access something in widgetList, I got a crash - it turns out my widgetList.widget was an invalid pointer... as if something had deleted it.

I went through this, and I've figured it out, but I thought I'd share since I think it's somewhat important/interesting.

[rollup="The Problem"]
When pushing back into a std::vector (or in fact, any other stl container), the container will create a copy of the object you tried to push back, and then destroy the original. The copy is a shallow copy, unless you've specified your own copy constructor.

So in my case above, pushing a ViewMember caused a copy to be created (which is fine), and then the original was destroyed, which called its constructor, which in turn deleted the pointer. Since the copy that was put in the vector was shallow, the pointer in that ViewMember is now pointing to invalid memory.
[/rollup]

[rollup="The Solution"]
1. Don't delete the pointer like that. Seems like a fairly bad way to handle this - now you have to manually delete the widget pointer.
2. Insert pointers in the std::vector instead of copied objects - pretty much the same, because now you have to manually delete the objects you inserted in the container.
3. Don't use pointer! Use std::shared_ptr instead! Best way - no manual memory management needed, can be copied without trouble, and won't delete its pointer if there's still something referencing it!
[/rollup]

0
Sign in to follow this  
Followers 0


0 Comments


There are no comments to display.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now