One Valid Case of Exception leaving a Destructor

Question

I'm working on a simple class to manage the lifetime of a HKEY.

class Key
{
    HKEY hWin32;
public:
    Key(HKEY root, const std::wstring& subKey, REGSAM samDesired);
    Key(const Key& other);
    ~Key();
    Key& operator=(const Key& other);
    Key& swap(Key& other);
    HKEY getRawHandle() { return hWin32; };
};

 //Other Methods....

Key::~Key()
{
    LONG errorCheck
        = RegCloseKey(hWin32);
    /*
     * I know it's generally bad to allow exceptions to leave destructors,
     * but I feel that if RegCloseKey() is going to fail, the application
     * should be terminated. (Because it should never fail.)
     */
    if (errorCheck != ERROR_SUCCESS)
        WindowsApiException::Throw(errorCheck);
}

Is this valid reasoning? I don't see how else a failure of RegCloseKey() can be communicated to the callee.

Answer 1

It's not generally bad. It is bad!

If an exception is thrown, the stack is unwound until a catch clause is met. If one of your Key objects is on the stack, it's destructor will be called. If it throws you have 2 exceptions at the same time. In this case terminate() is called so you won't get a chance to do anything with either one of your exceptions.

Answer 2

The failure of RegCloseKey is more of an assert situation than an error that needs to be passed up the call chain. You want to sit up and take notice right away in debug builds,

But what good is that failure information going to do the caller? What is he supposed to do about it?

Answer 3

Think about it this way - the reason ignoring an error message is bad is because if something does fail you should then be handling it and doing something with it (e.g. warn the user, retry, whatever). However in this case you're not really handling it either - you're just terminating the application (which is also bad practice).

I'd suggest that definitely having an assert to make sure that you catch anything in debug builds would be good. Writing something out using OutputDebugString might be a good idea for release builds. In that way you're not ignoring the app when an error occurs, you're noting it down but you're not terminating the app...

Terminating the app without warning or explanation is just going confuse and irritate your users.... How would you explain that to a user - "Well, something that I can't explain failed in a way that should never happen and I can't ignore it so I have to exit the application right away!"?

I guess my point is that leaking a handle, while bad, is much less bad than unexpected app shutdowns.