What will happen if a std::vector element 'commits suicide' (using delete this;)?

Question

Suppose there's a vector of Items

vector<Item*> items; //{item1, item2, item3}

Then, in other part of the code,

items[1]->suicide();

where the suicide function is:

void Item::suicide()
{
   delete this;
}

What is items vector size and how it's arrangement now? It is okay to do this?

Edit (may I ask an additional question?): If the desired arrangement of the output is {item1, item3}, size is 2, and no dangling pointer, how to do it in a self-destructing way (from the item2 itself)?

Edit 2 : Thanks for all the answers! Awesome. So I finally decided and found the way to do it from outside of the object because it was a bad practice and unnecessarily complicated

Kelsi

See related questions with the tag [self-destruction]: stackoverflow.com/questions/tagged/… – Bookmobile

Best thing you can do is to rethink your problem. Do you really need suicidal objects? Can you refactor it so that the object tells the caller whether it needs to be destroyed and then the caller deletes/removes from the vector? – Attendant

Answer 1

What is items vector size and how it's arrangement now? The same. The function call does not change the vector contents nor size at all. It just frees the memory the pointer is pointing to.

Is it okay to do this? More precisely: Is it legal C++? Yes. Is it good style programming? No. Let me elaborate on the latter:

• There should be a separation of concerns: Who's responsible for memory management? The container or the user of the class Item or the class Item itself?

• Typically the container or user should do that, because he knows what's going on.

• What's the way to do that? Memory management in modern and safe C++ code is mostly done using smart pointers like std::shared_ptr and std::unique_ptr and containers like std::vector and std::map.

• If the class Item is a value type (that means you can simply copy it and it has no polymorphic behavior in terms of virtual functions), then just use std::vector<Item> instead for your code. Destructors will be called automatically as soon as an element is removed from the container. The container does it for you.

• If the class Item has polymorphic behavior and can be used as a base class, then use std::vector<std::unique_ptr<Item>> or std::vector<std::shrared_ptr<Item>> instead and prefer the std::unique_ptr solution, because it adds less overhead. As soon as you stop referring to an object, it will be deleted automatically by the destructor of the smart pointer you are using. You (almost) don't need to worry about memory leaks anymore.

• The only way you can produce memory leaks is that you have objects that contain std::shared_ptrs that refer to each other in cyclic way. Using std::unique_ptrs can prevent this kind of trouble. Another way out are std::weak_ptrs.

Bottom line: Don't provide a function suicide(). Instead put the responsibility solely into the hands of the calling code. Use standard library containers and smart pointers to manage your memory.

Edit: Concerning the question in your edit. Just write

items.erase( items.begin() + 1 );

This will work for all types: std::vector of values or pointers. You can find a good documentation of std::vector and the C++ Standard library here.

Odle

Answer 2

The suicide member doesn't change the vector. So the vector contains an element which is an invalid pointer and formally you can't do much with an invalid pointer, even copying or comparing it is undefined behavior. So anything which access it, included vector resizing, is an UB.

While any access if formally UB, there is a good chance that your implementation doesn't behave strangely as long as you don't dereference the pointer -- the rationale for making any access UB is machines where loading an invalid pointer in a register can trap and while x86 is part of them, I don't know of widespread OS working in this mode.

Seaquake

Accessing a dangling pointer is not UB; dereferencing one is. – Blancheblanchette

@OliCharlesworth, I'd like a reference where the behavior is defined or at least left to the implementation. (I've never found one but I may have missed it, especially if it was introduced in C++11; loading an invalid segment descriptor is for sure a trap on x86 and such kind of thing has been mentioned as rationale the numerous time the UBness has been discussed on Usenet). – Seaquake

Loading it into an arbitrary register? – Blancheblanchette

Alternatively, if I'm wrong (and there's a non-zero chance that I am), then this would make a great question topic... ;) – Blancheblanchette

@OliCharlesworth, loading in a segment register. – Seaquake

But why would it ever be loaded into a segment register? – Blancheblanchette

Why not? Parameter passing convention asking that the segment part of pointers are passed in segment registers is the most obvious case which would be difficult to avoid. – Seaquake

This topic intrigues me, so I'll post a question once I've finished commuting. In the meantime, I think 6.2.6.1 p5 of the C99 standard is the key; presumably C++ will have similar semantics. – Blancheblanchette

FYI, I've now created this question. Please feel free to edit it if it doesn't correspond to your interpretation of things! – Blancheblanchette

In C++11 using an invalid pointer value causes UB under 3.7.4.2/4. In C++14 this was changed to implementation-defined (but with the proviso that the implementation may define a hardware trap). A deleted pointer is defined as being invalid under the same section. – Violoncellist

Answer 3

Your suicide function does not to anything with the Items vector, let alone it knows anything about it. So from the vector's point of view: nothing changes when you call the function and it's ok to do that.

Sultanate

From vector POV, the deleted pointer object now isn't in a valid state: it is no longer Assignable. The vector is in a special, almost invalid state. (But you can still destruct it, and you can still put it back in a valid state.) – Reg

that does not make sense. The vector is still in the exact same state. Not special, not invalid. Go check it. It's memory layout will not have changed. And a pointer is always assignable. And the vector has no real business whatsoever with the state of it's elements. I'm not advocating the OP should do this, but downvoting my answer is a bridge too far since it contains no false statements. – Sultanate

"The vector is still in the exact same state." No, the vector is broken (but fixable) because it now contains invalid objects. "And a pointer is always assignable." No, the pointer is invalid; all you can do is assign it a valid value. "but downvoting my answer is a bridge too far" I disagree with your answer. "since it contains no false statements" I claim it does. – Reg

"the vector is broken (but fixable) because it now contains invalid objects" please point me to the standard saying a vector exhibits undefined behaviour (which I assume you mean with broken) when one of the pointers it contains happens to point to a destructed object. You see, you can still safely use any method of such vector without anything nasty happenning. You can copy, resize, sort, whatever, since it just contains a POD which happens to be a pointer. That's not exactly what I would call broken. The only thing you cannot do without invoking UB is operations on the deleted element. – Sultanate

From n3242: [basic.stc.dynamic.deallocation]/4 "If the argument given to a deallocation function in the standard library is a pointer that is not the null pointer value (4.10), the deallocation function shall deallocate the storage referenced by the pointer, rendering invalid all pointers referring to any part of the deallocated storage. The effect of using an invalid pointer value (including passing it to a deallocation function) is undefined.3" – Reg

Do you also want a proof that vector is allowed to copy its elements? (I hope not; n3242 General containers requirement is such a mess.) – Reg

that only proves that calling items[1]->suicide(); followed by calling delete items[i] is UB. Not that deallocating the vector is UB. Or that copying a vector containing pointers that are invalid is UB. Or that the vector is somehow in a corrupted state. – Sultanate

let us continue this discussion in chat – Reg

@curiousguy: You keep mixing the vector element (a pointer) with the dereferenced vector element. – Llamas

Answer 4

The pointer will become invalid, that's all. You should be careful to not to delete it again. vector<Item*> will NOT delete elements on its own.

Verruca

To be concrete it will delete the memory where the pointers are stored but it wont delete the memory the pointers point to. – Gestation

@Dan: I believe you wrote it the other way around: delete will free the pointed memory, but not the pointer itself (i.e. it will leave the container untouched) – Attendant

Perhaps I was unclear, I meant the vector will clear its elements (the pointers) upon its destruction but not where they point. – Gestation

Answer 5

The vector has no idea what you're doing elsewhere in the code, so it'll keep a dangling pointer to the original Item.

"Is it OK do do that?"

After suiciding the item, you should adjust the vector manually to no longer keep that dangling pointer.

Llamas

..unless you want to directly replace it with another item – Sultanate

@stijn: "Replace" fits well within "Adjust" – Llamas

ah, semantics :] you're right though, I misunderstood it as "adjust the vector size" or so – Sultanate

@curiousguy: Either remove the item at the offending index alltogether (not recommended for a std::vector if extreme perf is an issue, since all trailing items would then be shifted up, essentially by copying a possibly big chunk of memory) or replace it with a valid item. Note that the Null Object Pattern can come in handy there, but YMMV. – Llamas

@JohannGerell After you delete the pointer, the vector is in invalid state, I don't think you can legally call erase. – Reg

@curiousguy: The std::vector<> has no idea that you've called delete on a pointer it's containing. How could it? The pointer you called delete on is still in the same spot in the vector, so either removing that std::vector<> item completely or replacing the value (the pointer) in that item is what you have to do. – Llamas

@JohannGerell After you can delete the pointer value becomes invalid. The container contains invalid (but destructible) elements. A container generally assumes its elements are valid. Destructing the container should be fine. "has no idea that you've called delete" That's the problem: the container still assume the elements are in a valid state. – Reg

@curiousguy: the container still assume the elements are in a valid state - No, it doesn't. It assumes no such thing. What makes you think that? If you call delete a second time on a pointer in a vector, then that is a bug on you. If you call delete on a pointer outside of a vector's context, then you must make sure that the vector item is also taken care of properly, as described above. There's no magic going on, you must take care of that yourself. – Llamas

@JohannGerell "It assumes no such thing." Of course it does. "What makes you think that?" It's spelled out in the standard. – Reg

@curiousguy: It's spelled out in the standard - I think you misunderstand the standard. The validity of an element has nothing to do with the validity of a the value of a dereferenced pointer element. The vector must be able to access the element at all times, otherwise you cannot use the vector. The semantics of a dereferenced element is completely unknown to the vector and is entirely up to you to handle. – Llamas

@JohannGerell No I read the standard just fine. I never said anything about dereferencing the elements - obviously they don't have to be dereferenceable! – Reg

Answer 6

That's ok in case of vector of pointers as vector will not call Item's destructor. But you have to somehow know which pointers are still valid.

If you are storing Items in vector by value, calling Item's destructor is not ok. When vector will be destroyed or cleared, it will call item's destructor again, causing application crash.

June

Answer 7

Wow, It seems that you make a typing error. It should be vector<Item *> Items; As to your question:

1. the size of vector Items does not change, means that, it still has three pointers to Item objects.
2. the content of the vector does not change: before Items[1]->suicide() , Items[0] = 0x000001, Items[1] = 0x000005, Items[2] = 0x000009 after Items[1]->suicide(), Items[0] = 0x000001, Items[1] = 0x000005, Items[2] = 0x000009
3. It's definitely OKAY to do so.

Besides, the vector will manage its memory automatically, when you push some elems into it while the capacity is not enough, it will reallocate a larger space, BUT, when you pop some elems or erase some elems, it will never give the redundant memory to the system.

The code of Items[1]->sucide() just return the memory held or pointed by the pointer Items[1] to the system, it will do nothing on the pointer itself, Items[1] still holds the same value, but point an unsafe area.

Unexpectedly, you have made a Design Pattern, suppose you want to design a class and you ONLY allow allocate any object of it on the Heap, you may write the following code:

class MustOnHeap
{
   private:
      ~MustOnHeap() { // ...}
   public:
      void suicide() { delete this;}

};

Then ,the class can not have any instance that is alloacated on the stack, because the destructor is private, and the compiler must arrange the calling of destructor when the object walk out its scope. Therefor, you must allocate them on the heap, MustOnHeap* p = new MustOnHeap; and then destroy it explicitly : p->suicide();

Palua