Freeing memory twice

Question

In C and C++, Freeing a NULL pointer will result in nothing done.

Still, I see people saying that memory corruption can occur if you "free memory twice".

Is this true? What is going on under the hood when you free memory twice?

Solution 1

int *p = malloc(sizeof(int));
//value of p is now lets say 0x12345678

*p = 2;
free(p); //memory pointer is freed, but still value of p is 0x12345678
         //now, if you free again, you get a crash or undefined behavior.

So, after free ing the first time, you should do p = NULL , so if (by any chance), free(p) is called again, nothing will happen.

Here is why freeing memory twice is undefined: Why free crashes when called twice

Solution 2

Freeing memory does not set the pointer to null. The pointer remains pointing to the memory it used to own, but which has now had ownership transferred back to the heap manager.

The heap manager may have since reallocated the memory your stale pointer is pointing to.

Freeing it again is not the same as saying free(NULL), and will result in undefined behavior.

Solution 3

This is undefined behavior, that can result in heap corruption or other severe consequences.

free() for a null pointer simply checks the pointer value inside and returns. That check will not help against freeing a block twice.

Here's what happens usually. The heap implementation gets the address and tries to "take ownership" of the block at that address by modifying its own service data. Depending on the heap implementation anything can happen. Maybe it works and nothing happens, maybe the service data is corrupted and you've got heap corruption.

So don't do it. It's undefined behavior. Whatever bad things can happen.

Solution 4

To avoid free twice i alway using MACRO for free memory:

#ifdef FREEIF
# undef FREEIF
#endif
#define FREEIF( _p )  \
if( _p )              \
{                     \
        free( _p );   \
        _p = NULL;    \
}

this macro set p = NULL to avoid dangling pointer.

Solution 5

Yes, "undefined behavior" which almost always results in a crash. (while "undefined behavior" by definition means "anything", various types of errors often behave in quite predictable ways. In case of free(), the behavior is invariably segfault or respective "memory protection error" characteristic to the OS.)

Same if you free() a pointer to anything else than NULL or something you malloc'd.

char x; char* p=&x; free(p); // crash.

View more solutions

Comments

[josesuero]

No memory is freed when you free a null pointer. If you free a non-null pointer twice, memory is freed twice, and that is a problem.

[bala.s]

I would also say explicitly that free ing a pointer doesn't (neceaarily) change it's value and in general it's not null after the 'free' (which is why the second free causes corruption).

[Admin]

You are too optimistic on the results of undefined behavior.

[David Rodríguez - dribeas]

+1 for correctness of the answer, even if I disagree in setting the pointer to NULL right after, that could hide a standing bug (calling free the second time) and I'd rather have it fail hard and fast so that the bug can be handled than hidden.

[N 1.1]

@David: I totally agree, if free is called twice on a pointer, it is a (somewhat) bad implementation.

[Janusz Lenar]

@David: That's a good strategy as long as you know how to debug 'free-twice' situations. If you don't, I'd rather nullify the pointer right after freeing and check it for nullness just before every free statement.

[Janusz Lenar]

@nvl: Yeah, that's why you call it a bug ;D

[N 1.1]

@malleor: doing your way, if you nullify pointer right after freeing, there is no need to check before free statement

[Admin]

I'm with Roger. In fact, freeing something twice almost never results in a crash at the point the free takes place, which makes it one of the most difficult bugs to track down.

[David Rodríguez - dribeas]

@malleor: having the double free kill the application you know that a pointer you are considering to be valid is actually not valid, hidding it can lead to situations where you dereference a null pointer and operate on it, which is undefined behavior (and depending on the system might just silently provide incorrect results). Debugging a crash is always simpler than debugging an incorrect value.

[Janusz Lenar]

@nvl: there is, if you want to find the bug - you're freeing twice! that was David's point.

[N 1.1]

@malleor: well, i would go by "debugging the crash" way.

[Janusz Lenar]

@David: well, i disagree, but that's a matter of preference. i prefer debugging online, e.g. after debug break inside CRT assertion, i.e. _ASSERTE(ptr!=NULL).

[jamesdlin]

@David: It's a tradeoff. I think leaving the pointer pointing to freed memory makes it more likely to hide errors where the pointer is used afterward, as dereferencing the null pointer is often more likely to crash. Double-freeing is undefined behavior as well, and it often won't lead to an immediate crash either.

[user206268]

This is a nice hack but I'd rather try to fix the actual cause. If free() get called twice on the same variable, this is clearly a bug.

[Vicky]

Setting p to NULL after freeing doesn't help with any of the other n pointers to that data you also had hanging around. So you still need to know how to debug a double-free.

[Shahbaz]

@N 1.1: if free is called twice, it is bad This isn't true. Imagine a kernel module that could be removed while initializing, or an object of a class being deleted from another thread (because the program is quiting) while it is still in the middle of initialization. In these cases, you may have temporary memory that needs to be cleaned up. This cleaning happens after initialization is done, but also needs to be done upon removing/deletion (because initialization may have been interrupted). Therefore, free needs to be called for the same memory twice. Hence, the set to '\0' solution.

[cmaster - reinstate monica]

@JanuszLenar I frankly don't understand why people are so fuzzy about setting this stupid pointer to NULL. In most situations it will go out of scope right after the free()/delete. Setting it to null between a free() and its passing into nonexistence is pretty pointless. And in those cases where the pointer does remain alive, you most likely need to set it to NULL anyway to signal that the memory doesn't exist anymore. So, this entire issue seems to get way more attention than it deserves.

[Janusz Lenar]

@cmaster Agreed. This is why the discussion was dead for a couple of years. Cheers, mate.