0

In the fallowing assignment operator I'm deleting a dynamically allocated data member of the Class I'm in. I accidentally left the "delete" in when turning the code in, but it still seems to work completely fine. What exactly is happening when I call delete and how is the swap function still working if I'm deleting m_itemArray before assigning it to temp in the Swap function?

Assignment operator:

Set& Set::operator=(const Set& rhs)
{
    if (this != &rhs)
    {
        delete[] m_itemArray;
        Set temp(rhs);
        swap(temp);
    }
    return *this;
}

Swap Function:

void Set::swap(Set& other)
{
    ItemType * temp = m_itemArray;
    m_itemArray = other.m_itemArray;
    other.m_itemArray = temp;
}

The way I'm thinking it works right now, is I'm letting the computer access the memory m_itemArray is using, but I'm getting lucky because the computer is not editing whatever is stored in that memory space by the time I'm accessing the Swap function.

Ash Bal
  • 9
  • 1
  • In short? Nothing happens. Why should a compiler let _happening_ anything there (in terms of efficiency).? – πάντα ῥεῖ Jan 24 '19 at 00:22
  • how am I able to set temp = m_itemArray if I deleted m_itemArray earlier? – Ash Bal Jan 24 '19 at 00:23
  • 3
    That's undefined behavior. – πάντα ῥεῖ Jan 24 '19 at 00:24
  • To add up: Asking about the behavior of _undefined behavior_ is quite futile and can't be answered definitely, since the behavior is ***undefined***. – πάντα ῥεῖ Jan 24 '19 at 00:31
  • Thanks. I understand its undefined behavior but I guess in a small program like mine I'm getting lucky that the values aren't being modified and the memory spaces aren't being used again. – Ash Bal Jan 24 '19 at 00:34
  • Take a look at the [Copy and Swap Idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) for a simpler and almost completely fool-proof way to implement `operator=`. Note that this solution can be overkill and a bit inefficient, but it makes a great starting point until it's proven to be too slow. – user4581301 Jan 24 '19 at 00:37
  • @AshBal _"but I guess in a small program like mine I'm getting lucky that the values aren't being modified and the memory spaces aren't being used again."_ You shouldn't rely on that anyways, so what? – πάντα ῥεῖ Jan 24 '19 at 00:38
  • 1
    A note on luck. This isn't it. Luck is when the program makes the mistake brutally obvious and forces you to fix it before the otherwise silently lurking bug manifests in the field and makes someone dead. – user4581301 Jan 24 '19 at 00:40
  • Yeah I guess luck would be a bad word. It's bad code I was just wondering why it wasn't throwing errors in my test cases. If I could turn it in again I would just leave the line out. – Ash Bal Jan 24 '19 at 00:42
  • "_I'm getting lucky_" More like, extremely unlucky as you could have missed it. – curiousguy Jan 24 '19 at 10:24

2 Answers2

1

Strictly speaking, what delete it is nothing; it's no longer owned, but until something else writes over it the values will still be there (for most allocators). However, trying to access memory that isn't allocated is undefined behaviour and anything can happen.

What exactly is happening when I call delete

The memory is released back to the manager.

how is the swap function still working if I'm deleting m_itemArray before assigning it to temp in the Swap function?

It just happens to still be there. The behaviour is undefined.

Daniel
  • 864
  • 5
  • 18
  • Thanks! So it does seem like I'm getting lucky that nothing is getting written into the memory I release by the time I use those values again in Swap. – Ash Bal Jan 24 '19 at 00:32
1

Your hypothesis is almost certainly correct. Except in very high security systems or when using debugging harnesses, there is no reason to rewrite the memory that has been released (which is what happens with the delete operation).

Actually, even the release itself might not occur immediately but rather be performed at a more convenient time (this operation can grow in complexity and become what is called a garbage collector, which in turn can render the use of explicit releases unnecessary - but the topic gets complicated).

So, until that moment, the memory is still reachable and may be used. Even afterwards, there is a possibility that the value is not reused and overwritten, and is still viable (which may contribute to hiding what is actually a dangerous bug). Of course, you have no guarantees, and trying to access freed memory can lead to all sorts of troubles.

To catch this kind of errors, with some systems you could link your executable with a different memory manager or library that will overwrite the soon-to-be-released memory with either random or telltale values before actually releasing it. With older systems that had no real hardware protection support, e.g. MS-DOS, this was the only check possible, and it remained a popular choice for quite some time (I remember electricfence, for example).

LSerni
  • 49,775
  • 9
  • 56
  • 97
  • Thanks! So in a small program like mine I most likely won't run into issues but it's still much cleaner to just get rid of the delete line in my assignment operator? – Ash Bal Jan 24 '19 at 00:31
  • 1
    @AshBal _"Thanks! So in a small program like mine I most likely won't run into issues"_ This is a serious issue, and you can't rely upon your observations. Undefined behavior is undefined behavior, full stop! Do it correctly instead. – πάντα ῥεῖ Jan 24 '19 at 00:44
  • 2
    c++ programs generally doesn't use a garbage collector (and in those few that do use it, you wouldn't call `delete` explicitly since the garbage collector is supposed to handle that for you). What the `delete` operator does (after executing the appropriate destructor-methods) is add a pointer to the no-longer-used block of memory back into the heap's available-memory-blocks list, so that the next time the program wants to allocate some memory (e.g. with the `new` operator), it can be handed that block of memory for re-use (if appropriate). – Jeremy Friesner Jan 24 '19 at 01:24