ExceptionsAndConstructors

#Designing constructors for exception safety

Prefer to be aware of problems related to exceptions in constructors.

Throwing exceptions from constructors is valid, and it's important. Exceptions are the only language mechanism to report failure from a constructor. Also, RAII principles dictate that resource allocation should occur in constructors. And since this is an important source of exceptions, we should expect that many exception come from constructors.

##Special problems with constructors There are some special rules for exceptions in constructors.

Consider the following:

TestObject::TestObject()
{
  SomeFunctionThatMayThrow(); // could get an exception here
  _memoryBlock = malloc(256);
}

TestObject::~TestObject()
{
  free(_memoryBlock);
}

In this case, we have an old-style explicit destruction of _memoryBlock in the destructor. _memoryBlock appears as if it will always be initialised to some valid value, but there is one problem case.

The example makes the problem obvious because SomeFunctionThatMayThrow is explicitly about exceptions. If that function throws, the constructor will be not fully completed. This will result in a partially constructed object, and one in which _memoryBlock has never been assigned a value.

Let's consider another example:

TestObject::TestObject()
: _member(SomeFunctionThatMayThrow())
, _memoryBlock(malloc(256))
{}

TestObject::~TestObject()
{
  free(_memoryBlock);
}

What happens if we get an exception in SomeFunctionThatMayThrow here? We haven't even reached the body of the constructor. The object is truly in an invalid state.

There is no way for class to know that _memoryBlock hasn't been assigned a value.

In this case, calling ~TestObject would be a disaster. There is no way for the destructor to know that the constructor threw an exception. And there is no way it can distinguish the random noise in _memoryBlock from some valid value.

So, the rule is this:

• If the constructor throws an exception,
• the destructor is not executed
• (however the memory object for the object will still be cleaned up. operator new() should free the block during unwinding)

##Resource leaks from constructor exceptions

Consider the following:

class ConfigFile
{
public:
  ConfigFile(const char name[]);
  ~ConfigFile();
private:
  FILE* _fileHandle;
};

ConfigFile::ConfigFile(const char filename[])
{
  _fileHandle = fopen(filename, "rb");
  unsigned version = 0;
  auto bytesRead = fread(&version, sizeof(unsigned), 1, _fileHandle);
  if (bytesRead != sizeof(unsigned) || version != 1) {
    ThrowException(::Exception::BasicLabel(
      StringMeld() << "Bad config file: " << filename));
  }
}

ConfigFile::~ConfigFile()
{
  fclose(_fileHandle);
}

Let's imagine we try to open a config file that exists on disk, but is 0 bytes long. In this case, the file will be opened correctly. But the fread operation will return 0. An exception will be triggered.

If the caller handles the exception, execution will continue. But the destructor for ConfigFile will never be called. This will mean that _fileHandle will not be closed, and this will become a leaked resource.

##Safer coding

How can we write this better? One way is to separate the file handling into another class:

class BasicFile
{
public:
  BasicFile(const char name[]);
  ~BasicFile();

  FILE* _fileHandle;
};

class ConfigFile
{
public:
  ConfigFile(const char name[]);
  ~ConfigFile();
private:
  std::unique_ptr<BasicFile> _file;
};

ConfigFile::ConfigFile(const char filename[])
: _file(std::make_unique<BasicFile>(filename))
{
  auto bytesRead = fread(&version, sizeof(unsigned), 1, _file->_fileHandle);
  if (bytesRead != sizeof(unsigned) || version != 1) {
    ThrowException(::Exception::BasicLabel(
      StringMeld() << "Bad config file: " << filename));
  }
}

ConfigFile::~ConfigFile()
{}

BasicFile::BasicFile(const char name[])
{
  _fileHandle = fopen(filename, "rb");
  if (!_fileHandle) {

      // It's safe to throw an exception here, because
      // we don't need the destructor to be called. If the
      // file didn't open correctly, we don't need to call
      // fopen.
      // In this particular case, even if the destructor is
      // not called, there will not be any leaks.

    ThrowException((::Exception::BasicLabel(
      StringMeld() << "Failed opening file: " << filename));
  }
}

BasicFile::~BasicFile()
{
  fclose(_fileHandle);
}

The above example will work without leaking, because of another special rule:

• Even though the destructor is not called,
• the destructors for members are!
• (also, destructors for base classes are called)

It's almost as if the compiler is automatically generating the following code in the constructor:

ConfigFile::ConfigFile(const char filename[])
: _file(std::make_unique<BasicFile>(filename))
{
  TRY {
    auto bytesRead = fread(&version, sizeof(unsigned), 1, _file->_fileHandle);
    if (bytesRead != sizeof(unsigned) || version != 1) {
      ThrowException(::Exception::BasicLabel(
        StringMeld() << "Bad config file: " << filename));
    }
  } CATCH (...) {
      // The compiler automatically adds in the following:
      // any members that were constructed before the exception
      // will have their destructor called
      // In the case of unique_ptr, this will result in it's
      // memory being freed
      // Since the compiler is automatically generating this
      // code, we shouldn't do this ourselves. This is just
      // an example. Running this code will probably cause a
      // double delete.
    _file.~std::unique_ptr<BasicFile>();
    this->~BaseClass();
    RETHROW;
  } CATCH_END
}

Another option is to write an explicit CATCH(...) block in the constructor, and make sure everything is cleaned up before exiting the constructor.

Conclusion

Be careful of exceptions that occur within constructors. When in doubt, consider added a CATCH(...) block and explicitly cleaning up everything that was constructed.