Friday, May 4, 2007

Structured Cleanup: Avoiding gotos

I think everyone in our profession has to write code, at some point, that grabs multiple resources, then do something.

And unconditionally, that code needs to make sure to back out any resource acquisitions in cases of intermediate failure, and only back out the resources that are actually grabbed.

Normally, you see this kind of code for that purpose:


int doStuff(void) {
char* a = (char*) malloc(100);
if (!a) goto failure_a;

char* b = (char*) malloc(100);
if (!b) goto failure_b;

// do some stuff

// Want to keep a and b around.
return 0;

// Future proofing, for when we add c.
free(b);
failure_b:
free(a);
failure_a:
return -1;
}


I have two problems with this code:
  1. Updating this code requires updating both the allocation semantics and the 'free' semantics.
  2. This doesn't lend itself towards any sort of 'pattern' which we can reuse for other things, like grabbing a mutex, semaphore, etc.
Instead of the above code, I propose that we use a system of Rollback objects. I haven't seen these in Design Patterns, but maybe they'll make their way into the next release of the book. :)


template <typename T>
class RollbackAllocator
{
RollbackAllocator(int countToAlloc) : mAllocated(0) { mAllocated = new T[countToAlloc]; }
~
RollbackAllocator() { if (mAllocated) delete [] mAllocated; }

// For easy-to-read client code, no one should take ownership of allocated
// objects until there are no further failure conditions possible. This same
// technique can be used for
// ANY resource acquisition, whether it's an allocation of memory,
// grabbing a socket, mutex, semaphore, reading a file, etc.
T* TakeOwnership() { T* retVal = mAllocated; mAllocated = 0; return retVal; }

T* mAllocated;
}

int doStuff(void)
{
// If any fails, it'll throw an exception and the others will be cleaned up.
RollbackAllocator<char> aAlloc(100);
RollbackAllocator<char> bAlloc(100);

// do additional things that could fail.

a = aAlloc.TakeOwnership();
b = bAlloc.TakeOwnership();

return 0;
}



The beauty of this code is its simplicity. By taking advantage of the guarantee that destructors will always be called upon exiting scope*, we ensure that a and b will always either be dealt with properly, or will be freed. Furthermore, while we do have to update the function in two places when we add a new resource acquisition, we do not have to worry about what order we TakeOwnership! Additionally, this pattern lends itself towards all sorts of resource acquisitions, whether they are mutex grabs, file reads, allocations as we've done above, etc. We can make this pattern fit virtually any resource acquisition pattern, and ensure that we have consistent, future-proof, goto-free code.

Don't get me wrong, gotos have their place. Just not in structured cleanup code.


* C++ doesn't always guarantee that destructors will be called. Any abnormal program termination, including explicit calls to exit, abort, terminate, pure virtual calls, exceptions thrown from exceptions (or destructors), or infinite loops will prevent destructors from being called.