82

Ignoring programming style and design, is it "safe" to call delete on a variable allocated on the stack?

For example:

   int nAmount;
   delete &nAmount;

or

class sample
{
public:
    sample();
    ~sample() { delete &nAmount;}
    int nAmount;
}
trincot
  • 317,000
  • 35
  • 244
  • 286
unistudent
  • 889
  • 1
  • 6
  • 5

12 Answers12

138

No, it is not safe to call delete on a stack-allocated variable. You should only call delete on things created by new.

  • For each malloc or calloc, there should be exactly one free.
  • For each new there should be exactly one delete.
  • For each new[] there should be exactly one delete[].
  • For each stack allocation, there should be no explicit freeing or deletion. The destructor is called automatically, where applicable.

In general, you cannot mix and match any of these, e.g. no free-ing or delete[]-ing a new object. Doing so results in undefined behavior.

Mr Fooz
  • 109,094
  • 6
  • 73
  • 101
  • 2
    Thanks! My compiler didn't seg fault but I was definitely suspicious if it was legitimate. – unistudent Jan 14 '09 at 03:50
  • 5
    "Should" is a better word. "Must" implies that the malloc/new/new[] will fail if the free/delete/delete[] is absent, which is not the case. The use of "exactly one" carries the implication I think you are going for. – Zooba Jan 14 '09 at 10:32
62

Well, let's try it:

jeremy@jeremy-desktop:~$ echo 'main() { int a; delete &a; }' > test.cpp
jeremy@jeremy-desktop:~$ g++ -o test test.cpp
jeremy@jeremy-desktop:~$ ./test
Segmentation fault

So apparently it is not safe at all.

Paige Ruten
  • 172,675
  • 36
  • 177
  • 197
  • 25
    I know this is an old answer, but I feel compelled to comment that trying something may not be the best way to prove that it is safe; if it worked, that would not imply that the operation is safe or that the behavior is well-defined, it would just prove that it worked *that one time*. (You can prove this way that things *don't work* but the inverse does not always hold.) – cdhowie Oct 08 '14 at 20:29
  • 22
    @cdhowie This is an irrelveant remark. Trying something and finding that it is unsafe *does* prove that it is unsafe. This is what the answer does. If the answerer hadn't found his example to segfault, he might have simply not posted it. You don't know how many things he tried before reaching that example. – jwg Nov 11 '15 at 17:30
  • 3
    @jwg I don't think you got cdhowies point. There are many things in C++ that are *not* safe but *can* work. I'm sure you can find a compiler that produces code that doesn't segfault because they thought it would be a great feature. Everything works just great. Now you switch compiler and notice that just because you didn't get an error with the previous compiler doesn't mean that your code isn't bad and unable to work with any other compiler. Computer science is more complex than 'It worked this time so it must be perfectly fine.' Not even to mention the concept of "Undefined Behaviour" – RecursiveExceptionException Jul 20 '17 at 21:52
  • 5
    @jwg To be fair, I think I did. – RecursiveExceptionException Jul 24 '17 at 22:18
18

Keep in mind that when you allocate a block of memory using new (or malloc for that matter), the actual block of memory allocated will be larger than what you asked for. The memory block will also contain some bookkeeping information so that when you free the block, it can easily be put back into the free pool and possibly be coalesced with adjacent free blocks.

When you try to free any memory that you didn't receive from new, that bookkeeping information wont be there but the system will act like it is and the results are going to be unpredictable (usually bad).

Ferruccio
  • 98,941
  • 38
  • 226
  • 299
13

Yes, it is undefined behavior: passing to delete anything that did not come from new is UB:

C++ standard, section 3.7.3.2.3: The value of the first argument supplied to one of thea deallocation functions provided in the standard library may be a null pointer value; if so, and if the deallocation function is one supplied in the standard library, the call to the deallocation function has no effect. Otherwise, the value supplied to operator delete(void*) in the standard library shall be one of the values returned by a previous invocation of either operator new(std::size_t) or operator new(std::size_t, const std::nothrow_t&) in the standard library.

The consequences of undefined behavior are, well, undefined. "Nothing happens" is as valid a consequence as anything else. However, it's usually "nothing happens right away": deallocating an invalid memory block may have severe consequences in subsequent calls to the allocator.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
8

After playing a bit with g++ 4.4 in windows, I got very interesting results:

  1. calling delete on a stack variable doesn't seem to do anything. No errors throw, but I can access the variable without problems after deletion.

  2. Having a class with a method with delete this successfully deletes the object if it is allocated in the heap, but not if it is allocated in the stack (if it is in the stack, nothing happens).

Sambatyon
  • 3,316
  • 10
  • 48
  • 65
  • 4
    Your answer is actually relevant to the question. There are always too many evangelist programmers on SO condemning anyone who asks a question out of their own sheer curiosity (the reason I'm here) as to what the standard is that should occur in unexpected corner cases of a language. – Joey Carson Apr 03 '14 at 20:19
6

Nobody can know what happens. This invokes undefined behavior, so literally anything can happen. Don't do this.

  • 1
    OK, supposedly I have a hierarchy tree of parent/child objects. The tree is responsible for recursively invoking different object methods and upon deletion of a root object all children should get recursively deleted. However some of the children may be scope allocated, others dynamically, in this case it is not possible to distinguish between the two and only delete dynamic objects without adding some more data to specify it? – dtech Sep 20 '12 at 13:23
  • 1
    @ddriver: No, it's not possible to get that information given just a pointer. You could use `std::shared_ptr` with a custom deleter that does nothing to non-dynamic objects, or add your own metadata, or come up with a less convoluted lifetime management strategy. – Mike Seymour Sep 20 '12 at 13:27
  • @ddriver: I usually go by a simple rule: The object or scope that is responsible for an object creation is also responsible for its deletion. Or put it otherwise: don't `delete` an object you haven't `new`ed in the first place. – ereOn Sep 20 '12 at 13:32
  • 2
    @ddriver if you need this, your design is seriously flawed. Rethink it, this should not be a problem. –  Sep 20 '12 at 13:32
  • @MikeSeymour, rest - I am asking mostly because I am curious of how this scheme works in Qt, where QObject can accept both stack and heap objects for children, and supposedly when the parent is deleted, it goes through and deletes the entire tree. And the tree itself is not only designed for lifetime management, there is all kind of functionality that can be ran on the tree recursively. – dtech Sep 20 '12 at 13:35
  • "and supposedly" - so not actually. –  Sep 20 '12 at 13:36
  • Supposedly as in possibly, not as in "not actually" - that is what the documentation says. "The parent takes ownership of the object; i.e., it will automatically delete its children in its destructor." – dtech Sep 20 '12 at 13:37
  • @ddriver oh, that's different. (For me, supposedly means that you think it is, sorry.) –  Sep 20 '12 at 13:39
  • Maybe I will have better chances for an answer if I move it to a new question and focus on that instead of starting at the bottom, i.e. what happens when calling `delete` on a stack object. – dtech Sep 20 '12 at 13:40
  • 2
    @ddriver: Where did you get the QObject stuff from? In the Qt API docs, at [QObject::~QObject()](http://qt-project.org/doc/qt-4.8/qobject.html#dtor.QObject), it clearly says: _Warning: All child objects are deleted. If any of these objects are on the stack or global, sooner or later your program will crash._ – Andreas Fester Sep 20 '12 at 13:50
4

No, Memory allocated using new should be deleted using delete operator and that allocated using malloc should be deleted using free. And no need to deallocate the variable which are allocated on stack.

Vinay
  • 4,743
  • 7
  • 33
  • 43
3

An angel loses its wings... You can only call delete on a pointer allocated with new, otherwise you get undefined behavior.

Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
1

here the memory is allocated using stack so no need to delete it exernally but if you have allcoted dynamically

like int *a=new int()

then you have to do delete a and not delete &a(a itself is a pointer), because the memory is allocated from free store.

0

You already answered the question yourself. delete must only be used for pointers optained through new. Doing anything else is plain and simple undefined behaviour.

Therefore there is really no saying what happens, anything from the code working fine through crashing to erasing your harddrive is a valid outcome of doing this. So please never do this.

Grizzly
  • 19,595
  • 4
  • 60
  • 78
0

It's UB because you must not call delete on an item that has not been dynamically allocated with new. It's that simple.

Martin James
  • 24,453
  • 3
  • 36
  • 60
0

Motivation: I have two objects, A and B. I know that A has to be instantiated before B, maybe because B needs information calculated by A. Yet, I want to destruct A before B. Maybe I am writing an integration test, and I want server A to shut-down first. How do I accomplish that?

A a{};
B b{a.port()};

// delete A, how?

Solution: Don't allocate A on the stack. Instead, use std::make_unique and keep a stack-allocated smart pointer to a heap-allocated instance of A. That way is the least messy option, IMO.

auto a = std::make_unique<A>();
B b{a->port()};

// ...

a.reset()

Alternatively, I considered moving the destruction logic out of A's destructor and calling that method explicitly myself. The destructor would then call it only if it has not been called previously.

user7610
  • 25,267
  • 15
  • 124
  • 150