Don’t try for exception safety Thursday 20th December 2007

Achieve it!

“Do or do not, there is no try” – Yoda, on strong exception safety.

I’ve decided that I like exceptions a lot more now that I know how to use them to my advantage. Take the following piece of code:


#include <cstring>
using std::memcmp;

size_t DoConversion( char* dst, size_t dst_len, const char* src, size_t src_len );
size_t DoConversion2( char* dst, size_t dst_len, const char* src, size_t src_len );

void test_conv( const char* test_data, size_t test_len )
{
        size_t bufsize = DoConversion( NULL, 0, test_data, test_len );

        char* buffer = new char[ bufsize ];

        DoConversion( buffer, bufsize, test_data, test_len );

        size_t bufsize2 = DoConversion2( NULL, 0, buffer, bufsize );

        if( bufsize2 != bufsize )
                throw TestFailed();

        char* buffer2 = new char[ bufsize2 ];

        DoConversion2( buffer2, bufsize2, buffer, bufsize );

        if( memcmp( buffer, buffer2, bufsize ) != 0 )
                throw TestFailed();

        delete[] buffer2;

        delete[] buffer;
}

Assume that DoConversion and DoConversion2 are “traditional” character string conversion functions. They take a source and destination buffer which they don’t memory manage and convert one to the other. If you supply a null destination buffer then they will tell you how big the destination buffer would have to be to complete the conversion without actually performing the conversion. Assume that they are less traditional in that they may throw a BadThing exception if something doesn’t work.

The test_conv function is obviously not exception safe, and in multiple ways. Trying to make it exception safe in a naive way – by adding some try/catch pairs – is verbose and error prone. I came up with this, but I have low confidence in the result (and I actually know of one definite reason why it is not exception safe).

void test_conv( const char* test_data, size_t test_len )
{
        size_t bufsize = DoConversion( NULL, 0, test_data, test_len );

        char* buffer = new char[ bufsize ];

        size_t bufsize2;

        try
        {
                DoConversion( buffer, bufsize, test_data, test_len );

                bufsize2 = DoConversion2( NULL, 0, buffer, bufsize );
        }
        catch( ... )
        {
                delete[] buffer;
                throw;
        }

        if( bufsize2 != bufsize )
        {
                delete[] buffer;
                throw TestFailed();
        }

        char* buffer2 = new char[ bufsize2 ];

        try
        {
                DoConversion2( buffer2, bufsize2, buffer, bufsize );
        }
        catch( ... )
        {
                delete[] buffer2;
                delete[] buffer;
                throw;
        }

        int res = memcmp( buffer, buffer2, bufsize );

        delete[] buffer2;
        delete[] buffer;

        if( res != 0 )
                throw TestFailed();
}

This is ugly in so many ways. buffer is allocated in one place and deallocated in one of four places (or even not at all!), depending on the particular path followed; bufsize2 now has to be declared before we can sensibly initialize it; the result of memcmp is cached so that deallocation can take place before deciding whether to throw or not (this was mildy shorter that duplicating the two delete statements yet again).

So here’s the answer: write a new class.

class AutoCharArray
{
public:
        AutoCharArray( size_t s ) : _buffer( new char[s] ) {}

        ~AutoCharArray() { delete[] _buffer; }

        operator char*() const { return _buffer; }

private:
        // No copying
        AutoCharArray( const AutoCharArray& );
        AutoCharArray& operator=( const AutoCharArray& );

        char* _buffer;
};

void test_conv( const char* test_data, size_t test_len )
{
        size_t bufsize = DoConversion( NULL, 0, test_data, test_len );

        AutoCharArray buffer( bufsize );

        DoConversion( buffer, bufsize, test_data, test_len );

        size_t bufsize2 = DoConversion2( NULL, 0, buffer, bufsize );

        if( bufsize2 != bufsize )
                throw TestFailed();

        AutoCharArray buffer2( bufsize2 );

        DoConversion2( buffer2, bufsize2, buffer, bufsize );

        if( memcmp( buffer, buffer2, bufsize ) != 0 )
                throw TestFailed();

}

AutoCharArray just manages the life time of the dynamically allocated char array as a C++ object. Because of this, we never have to worry about catching an rethrowing foreign exceptions. Because it is a C++ object, if it has been successfully constructed as a local object then it will be destroyed when the function exits, whether conventionally or via an exception. We don’t even have to worry about “new” failing. If new throws, the constructor will not have completed so the destructor will not be called on a bad pointer.

As well of all these advantages, the control flow for the optimistic ‘success’ use case is obvious and easy to follow. It is not cluttered with a ton of “but in case that didn’t work” catch blocks. Overall, including the class definition, the entire code is no longer than the long winded “try/catch” quagmire of the first attempt.

A simple “AutoArray” class is very useful for this type of application, although I tend to prefer it as a template:

template< class T >
class AutoArray
{
public:
        AutoArray( size_t s ) : _buffer( new T[s] ) {}

        ~AutoArray() { delete[] _buffer; }

        operator T*() const { return _buffer; }

private:
        // No copying
        AutoArray( const AutoArray& );
        AutoArray& operator=( const AutoArray& );

        T* _buffer;
};
3 Comments
Darren December 21st, 2007

Forgive my ignorance, but doesn’t C++ have a “finally” clause to deal with the deallocation?

Pseudocde:
try
{
// allocate memory
}
catch
{
// log error
}
finally
{
// deallocate
}

Note that finally is always called, whether exceptions are thrown or not.

That’s how it works in Java-land and they based it all on C++ right?

Or have I misread it and you only want it deallocated if there is an exception?

Steve December 21st, 2007

I was so nearly tempted to recommend std::auto_ptr, but of course that doesn’t cope well at all with arrays. Herb Sutter has a GotW dealing with exactly this topic: why not just use std::vector?

hashpling December 21st, 2007

There is no ‘finally’ in C++, it’s just Java, AFAIK.

And yes, you could just use a std::vector< char > as it’s guaranteed that vector elements are stored contiguously. For some reason I feel that relying on this guaranteed “implementation detail” is a bit too intrusive, but I’m not sure why I feel that way as it is a guarantee that can be usefully relied on.

So you could have:

std::vector buffer( bufsize );

and:

DoConversion( &buffer[0], bufsize, test_data, test_len );

Anyway, in the next version of C++ we will get std::unique_ptr to replace auto_ptr which will have a customizable deleter and so will support correctly deleting dynamically allocated arrays.