Delete an object from a vector

Question

Step 1. Create an instance of a class

Step 2. Push this instance to a vector

Step 3. Call delete this; in a member method of an instance

Step 4. Everything is Ok

Step 5. Push something to the vector and get this

*** glibc detected *** ./app: double free or corruption (fasttop): 0x0000000001017930 ***
======= Backtrace: =========
/lib/libc.so.6(+0x71bd6)[0x7f607d60cbd6]
/lib/libc.so.6(cfree+0x6c)[0x7f607d61194c]
./app[0x40231c]
./app[0x402290]
./app[0x4053c0]
./app[0x4048fe]
./app[0x404246]
./app[0x403fe0]
./app[0x402400]
./app[0x4035cb]
./app[0x4034d3]
/lib/libpthread.so.0(+0x68ca)[0x7f607e2b78ca]
/lib/libc.so.6(clone+0x6d)[0x7f607d66a92d]
======= Memory map: ========
00400000-0040f000 r-xp 00000000 09:03 60427370                           /root/AHS/app
0060e000-0060f000 rw-p 0000e000 09:03 60427370                           /root/AHS/app
01017000-01038000 rw-p 00000000 00:00 0                                  [heap]
7f6074000000-7f6074021000 rw-p 00000000 00:00 0
7f6074021000-7f6078000000 ---p 00000000 00:00 0
7f607a595000-7f607a596000 ---p 00000000 00:00 0
7f607a596000-7f607ad96000 rw-p 00000000 00:00 0
7f607ad96000-7f607ad97000 ---p 00000000 00:00 0
7f607ad97000-7f607b597000 rw-p 00000000 00:00 0
7f607b597000-7f607b598000 ---p 00000000 00:00 0
7f607b598000-7f607bd98000 rw-p 00000000 00:00 0
7f607bd98000-7f607bd99000 ---p 00000000 00:00 0
7f607bd99000-7f607c599000 rw-p 00000000 00:00 0
7f607c599000-7f607c59a000 ---p 00000000 00:00 0
7f607c59a000-7f607cd9a000 rw-p 00000000 00:00 0
7f607cd9a000-7f607cd9b000 ---p 00000000 00:00 0
7f607cd9b000-7f607d59b000 rw-p 00000000 00:00 0
7f607d59b000-7f607d6f4000 r-xp 00000000 09:03 60425052                   /lib/libc-2.11.3.so
7f607d6f4000-7f607d8f3000 ---p 00159000 09:03 60425052                   /lib/libc-2.11.3.so
7f607d8f3000-7f607d8f7000 r--p 00158000 09:03 60425052                   /lib/libc-2.11.3.so
7f607d8f7000-7f607d8f8000 rw-p 0015c000 09:03 60425052                   /lib/libc-2.11.3.so
7f607d8f8000-7f607d8fd000 rw-p 00000000 00:00 0
7f607d8fd000-7f607d913000 r-xp 00000000 09:03 60425245                   /lib/libgcc_s.so.1
7f607d913000-7f607db12000 ---p 00016000 09:03 60425245                   /lib/libgcc_s.so.1
7f607db12000-7f607db13000 rw-p 00015000 09:03 60425245                   /lib/libgcc_s.so.1
7f607db13000-7f607db93000 r-xp 00000000 09:03 60425438                   /lib/libm-2.11.3.so
7f607db93000-7f607dd93000 ---p 00080000 09:03 60425438                   /lib/libm-2.11.3.so
7f607dd93000-7f607dd94000 r--p 00080000 09:03 60425438                   /lib/libm-2.11.3.so
7f607dd94000-7f607dd95000 rw-p 00081000 09:03 60425438                   /lib/libm-2.11.3.so
7f607dd95000-7f607de8b000 r-xp 00000000 09:03 60032880                   /usr/lib/libstdc++.so.6.0.13
7f607de8b000-7f607e08b000 ---p 000f6000 09:03 60032880                   /usr/lib/libstdc++.so.6.0.13
7f607e08b000-7f607e092000 r--p 000f6000 09:03 60032880                   /usr/lib/libstdc++.so.6.0.13
7f607e092000-7f607e094000 rw-p 000fd000 09:03 60032880                   /usr/lib/libstdc++.so.6.0.13
7f607e094000-7f607e0a9000 rw-p 00000000 00:00 0
7f607e0a9000-7f607e0b0000 r-xp 00000000 09:03 60425177                   /lib/librt-2.11.3.so
7f607e0b0000-7f607e2af000 ---p 00007000 09:03 60425177                   /lib/librt-2.11.3.so
7f607e2af000-7f607e2b0000 r--p 00006000 09:03 60425177                   /lib/librt-2.11.3.so
7f607e2b0000-7f607e2b1000 rw-p 00007000 09:03 60425177                   /lib/librt-2.11.3.so
7f607e2b1000-7f607e2c8000 r-xp 00000000 09:03 60425205                   /lib/libpthread-2.11.3.so
7f607e2c8000-7f607e4c7000 ---p 00017000 09:03 60425205                   /lib/libpthread-2.11.3.so
7f607e4c7000-7f607e4c8000 r--p 00016000 09:03 60425205                   /lib/libpthread-2.11.3.so
7f607e4c8000-7f607e4c9000 rw-p 00017000 09:03 60425205                   /lib/libpthread-2.11.3.so
7f607e4c9000-7f607e4cd000 rw-p 00000000 00:00 0
7f607e4cd000-7f607e4eb000 r-xp 00000000 09:03 60425293                   /lib/ld-2.11.3.so
7f607e6da000-7f607e6df000 rw-p 00000000 00:00 0
7f607e6e7000-7f607e6ea000 rw-p 00000000 00:00 0
7f607e6ea000-7f607e6eb000 r--p 0001d000 09:03 60425293                   /lib/ld-2.11.3.so
7f607e6eb000-7f607e6ec000 rw-p 0001e000 09:03 60425293                   /lib/ld-2.11.3.so
7f607e6ec000-7f607e6ed000 rw-p 00000000 00:00 0
7fff4ee3b000-7fff4ee50000 rw-p 00000000 00:00 0                          [stack]
7fff4efff000-7fff4f000000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
Aborted

Could someone please tell me, what is this, why this occurs and how do I fix it?

`delete this` is a code smell. It has its uses, but they are few and far between. — Chad, Aug 23 '12 at 21:18
Why do you want to `delete this`? It's almost certainly a bad idea. — Beta, Aug 23 '12 at 21:19
Some events happen inside the instance, after which it must be deleted. How do I delete it properly? — Kolyunya, Aug 23 '12 at 21:24
Rule of thumb: Have exactly one `delete` for each `new` and exactly one `delete[]` for each `new[]`. You have a `delete` in your code with no matching `new`. Havoc ensues. — Robᵩ, Aug 23 '12 at 21:27
`delete this` is sawing off the branch that you're sitting on. — Roddy, Aug 23 '12 at 21:29
Keep in mind that when you push a thing onto a standard vector, the thing gets copy constructed. The class instance you created, and the one in the vector, are different ones. — Ari, Aug 23 '12 at 21:31

score 4 · Answer 1 · answered Aug 23 '12 at 21:19

4

It's because you freed memory you didn't own. The vector owns the memory containing its contents.

delete this; is like taking a rental car to the salvage yard when you're done with it. Don't do that, the rental company expects it back!

Be careful of the difference between owning memory, and merely having control of it loaned to you.

answered Aug 23 '12 at 21:19

Ben Voigt

277,958
43
419
720

And how I am supposed to delete the instance? Let's say, I check some condition inside an instance, if it is `true`, this instance must be deleted. – Kolyunya Aug 23 '12 at 21:26
@Kolyunya: You don't need to delete the instance. If you want it removed from the vector, then remove it from the vector. – Ben Voigt Aug 23 '12 at 21:31
`just use the vector::erase method` how do I call it from inside of the instance which is a member of that vector? Am I supposed to pass the pointer to the vector to that instance? – Kolyunya Aug 23 '12 at 21:34

score 2 · Accepted Answer · answered Aug 23 '12 at 21:24

2

Doing delete this for an object belonging to a vector you are making two big mistakes:

it's the vector that manages the memory where that instance is stored (it allocates it, deallocates it, ...), so you shouldn't mess with it. In other words, since you don't own that memory you must leave it alone.
delete is a correct match if the object was allocated with new - but the object managed by the vector is not allocated like that; instead, vector grabs a chunk of memory from the allocator (as big as it thinks it's sensible) and copies there the elements that are pushed into it using placement new; so, not only you are freeing memory that you don't own, you are also using the wrong method to free it.

If you need to remove an element from a vector, just use the vector::erase method.

answered Aug 23 '12 at 21:24

Matteo Italia

123,740
17
206
299

`just use the vector::erase method` how do I call it being inside of the instance which is a member of the vector??? Am I supposed to pass the pointer to the vector to that instance? – Kolyunya Aug 23 '12 at 21:33
@Kolyunya: it depends on what exactly you are trying to achieve; you should explain a little more the "big picture" of what you are doing. My first thought would be to have a "manager" class that encapsulates the vector and acts as a proxy for the requests to the objects; the object may return a flag that the external class will use to determine if the object must be erased. Since this is done "from the outside", there's no need for the object to know exactly how to "commit suicide" (which, by the way, is usually considered "code smell"). – Matteo Italia Aug 23 '12 at 21:37
I've a game server. It creates `game` which are instances of `Game` and adds it to the vector. After that some events happen inside of the `game` e. g. _game_ends_. At this very moment the `game` should be deleted from the vector and destroyed... As I can see, you advise me to make an `observer` pattern, right? `game` dispatches event `delete` and the server, which is an observer deletes it. Yes? – Kolyunya Aug 23 '12 at 21:47
@Kolyunya: it's something like that; I would define a `GamesManager` class that handles all the game creation/destruction stuff; how to notify the `GamesManager` that a game must be erased depends on how exactly your code is structured. Also, to avoid continued copying, you may simply put games in "ended" state and cyclically perform a check over the whole vector, removing ended games and "defragmenting" the vector. – Matteo Italia Aug 23 '12 at 21:54
Thanks for idea! `cyclically perform a check over the whole vector, removing ended games` – Kolyunya Aug 23 '12 at 21:57
@Kolyunya: ironically, it's a bit like implementing your little GC in C++ :) . By the way, if your game objects are expensive to copy other data structures (e.g. `std::set` or a vector of pointers) may perform better. – Matteo Italia Aug 23 '12 at 22:00
Don't understand about GC =\ In fact `games` ARE expensive to copy... Maybe I should store pointers to them? What will it change? I'll be able to call `delete this`? – Kolyunya Aug 23 '12 at 22:04

Chip · Answer 3 · 2012-08-23T21:33:18.237

1

If you do delete this in any method, you have to be sure that no one else calls any method ( for that matter any code) of the instance after that statement. This includes the destructor.

When you push the instance into a vector, while being destroyed, the vector calls the destructor for the instance, hence the double free

If you need a vector, you can push pointers to the instance and it would be fine.

However, as others have said DO NOT use delete this unless absolutely necessary

You would also have the problems if you just create an local instance of the class in a function. Most likely you did not see this behavior because your program finished before the scope of the local variable ended. If you tried this :

void func() {
    MyClass myClass;
    myClass.theBadFunc();
}

When this returns, you will have a core dump.

edited Aug 23 '12 at 21:33

answered Aug 23 '12 at 21:26

Chip

3,226
23
31

I don't get it `When you push the instance into a vector, while being destroyed, the vector calls the destructor for the instance` Why does vector call a d-structor, when I push an element? – Kolyunya Aug 23 '12 at 21:31
The vector will only call the destructor of each element when it is destroyed i.e. it goes out of scope. – Chip Aug 23 '12 at 21:34
@Kolyunya what problem are you trying to solve when you are doing `delete this`? – Chip Aug 23 '12 at 21:35
Ok, the problem is I create an instance of class and push it to the vector. Instance _lives_it_life_independently_. I mean, it regulates itself. Then THE MOMENT COMES and I need to delete the instance from the vector. BUT! I cant't call `vector.erase()` from the instance, coz my instance knows nothing about this vector! The only way to solve this is to pass a pointer-to-vector to the instance. Am I right? – Kolyunya Aug 23 '12 at 21:42
You are absolutely right. You should not call `vector.erase()`. The first problem is that the vector actually **copies** your instance. This may or may not be the behavior you want. You have a couple of options. Let me add to my answer. – Chip Aug 23 '12 at 21:49
@Kolyunya Actually, Can you ask a separate question on this? Describe your case and post the link here and I will try and answer it. – Chip Aug 23 '12 at 21:52
The case is: I've got a game server. It creates `game` which are instances of `Game` and adds it to the vector. After that some events happen inside of the `game` e. g. `game_ends`. At this very moment the `game` should be deleted from the vector... What is the best way to achieve it? Thanks much! – Kolyunya Aug 23 '12 at 21:55
Ah.. for this case, you should never use `delete this` from within the Game class, because this doesn't even delete it from the vector. Which class has the vector? that is the class that should call erase on the vector. Please ask this question as a **separate question**. You will get a better response and I can give you code examples. – Chip Aug 23 '12 at 22:02
What do I ask in the new question? It will be a certain duplicate and I will be banned... – Kolyunya Aug 23 '12 at 22:09
You will not be banned. You can ask (quoting you) "I've got a game server. It creates game which are instances of Game and adds it to the vector. After that some events happen inside of the game e. g. game_ends. At this very moment the game should be deleted from the vector... What is the best way to achieve it?" This is a totally different question that "Why does `delete this` cause an core dump" – Chip Aug 23 '12 at 22:13

score 1 · Answer 4 · answered Aug 23 '12 at 21:37

Does this resemble the code that you're describing?

struct MyClass {
    void f() { delete this; }
};

int main() {
    MyClass c;
    std::vector<MyClass> v;
    v.push_back(c);
    c.f();
    return 0;
}

The vector is irrelevant. The problem is that delete this calls the destructor for c and frees the memory where c was built. That memory is on the stack, and cannot be freed. Don't ever delete stack objects. The compiler generates code to clean them up when they go out of scope, in this case, at the end of main.

Delete an object from a vector

4 Answers4

Linked