Dangling pointers and double free

Question

After some painful experiences, I understand the problem of dangling pointers and double free. I am seeking proper solutions. aStruct has a number of fields including other arrays.

aStruct *A=NULL, *B = NULL;
A = (aStruct*) calloc(1, sizeof(sStruct));
B = A;
free_aStruct(A);
...
//bunch of other code in various places.
...
free_aStruct(B);

Is there any way to write free_aStruct(X) so that free_aStruct(B) exists gracefully??

void free_aStruct(aStruct *X){
  if (X ! = NULL){
      if (X->a != NULL){free(X->a); x->a = NULL;}
      free(X); X = NULL;
}
}

Doing above only sets A = NULL when free_aStruct(A); is called. B is now dangling. How can this situation be avoided / remedied? Is reference counting the only viable solution? or, are there other "defensive" free approaches, to prevent free_aStruct(B); from exploding?

Thanks,

Russ

Answer 1

Even if you could prevent the free_aStruct(B) from blowing up, if there's any reference to B in the code behind your comment, that's going to be using memory that's been freed, and so might be overwritten with new data at any point. Just "fixing" the free call will only mask the underlying error.

Answer 2

In plain C, the most important solution to this problem is discipline, because the root of the problem is here:

B = A;

Making a copy of the pointer without changing anything within your struct, circumventing whatever you use without any warning from the compiler. You have to use something like this:

B = getref_aStruct(A);

The next important thing is to keep track of the allocations. Some things that help are clean modularization, information hiding and DRY -- Don't Repeat Yourself. You directly call calloc() to allocate the memory while you use a free_aStruct() function to free it. Better use a create_aStruct() to allocate it. This keeps things centralized and in one place only, instead of throwing memory allocations all over your codebase.

This is a much better base for whatever memory tracking system you build on top of this.

Answer 3

There are techniques you can use but the bottom line is that nothing you do can be strictly enforcable in C. Instead, i recommend incorporating valgrind (or purify) in your development process. Also, some static code analyzers may be able to detect some of these problems.

Answer 4

Reference counting's really not that hard:

aStruct *astruct_getref(aStruct *m)
{
    m->refs++;
    return m;
}

aStruct *astruct_new(void)
{
    sStruct *new = calloc(1, sizeof *new);
    return astruct_getref(new);
}

void astruct_free(aStruct *m)
{
    if (--m->refs == 0)
        free(m);
}

(In a multithreaded environment you will also potentially need to add locking).

Then your code would be:

aStruct *A = NULL, *B = NULL;
A = astruct_new();
B = astruct_getref(A);
astruct_free(A);
...
//bunch of other code in various places.
...
astruct_free(B);

---

You've asked about locking. Unfortunately there's no one-size-fits-all answer when it comes to locking - it all depends on what access patterns you have in your application. There's no substitute for careful design and deep thoughts. (For example, if you can guarantee that no thread will be calling astruct_getref() or astruct_free() on another thread's aStruct, then the reference count doesn't need to be protected at all - the simple implementation above will suffice).

That said, the above primitives can easily be extended to support concurrent access to the astruct_getref() and astruct_free() functions:

aStruct *astruct_getref(aStruct *m)
{
    mutex_lock(m->reflock);
    m->refs++;
    mutex_unlock(m->reflock);
    return m;
}

aStruct *astruct_new(void)
{
    sStruct *new = calloc(1, sizeof *new);
    mutex_init(new->reflock);
    return astruct_getref(new);
}

void astruct_free(aStruct *m)
{
    int refs;

    mutex_lock(m->reflock);
    refs = --m->refs;
    mutex_unlock(m->reflock);
    if (refs == 0)
        free(m);
}

...but note that any variables containing the pointers to the structs that are subject to concurrent access will need their own locking too (for example, if you have a global aStruct *foo that is concurrently accessed, it will need an accompanying foo_lock).