August 7th, 2014

Since clean-up functions can't fail, you have to soldier on

Clean-up functions can’t fail, so what do you do if you encounter a failure in your clean-up function?

You have to keep cleaning up.

Some people like to follow this pattern for error checking:

HRESULT Function()
{
  hr = SomeFunction();
  if (FAILED(hr))) goto Exit;
  hr = AnotherFunction();
  if (FAILED(hr))) goto Exit;
  ... and so on until ...
  hr = S_OK;
Exit:
    return hr;
}

And some like to put it inside a cute flow control macro like

#define CHECK_HRESULT(hr) if (FAILED(hr)) goto Exit;

or even

#define CHECK_HRESULT(f) if (FAILED(hr = (f))) goto Exit;

Whatever floats your boat.

But you have to be careful if using this pattern in a clean-up function, because you might end up not actually cleaning up. For example:

HRESULT Widget::Close()
{
    HRESULT hr;
    CHECK_HRESULT(DisconnectDoodad(m_hDoodad));
    m_hDoodad = nullptr;
    for (int i = 0; i < ARRRAYSIZE(GadgetArray); i++) {
        CHECK_HRESULT(DestroyGadget(m_rghGadget[i]));
        m_rghGadget[i] = nullptr;
    }
    hr = S_OK;
Exit:
    return hr;
}

What if there is an error disconnecting the doodad? (Maybe you got RPC_E_SERVER_DIED because the doodad lives on a remote server which crashed.) The cleanup code treats this as an error and skips destroying the gadget. But what can the caller do about this? Nothing, that’s what. Eventually you get a bug that says, “On an unreliable network, we leak gadgets like crazy.”

Or worse, what if you’re doing this in your destructor. You have nowhere to report the error. The caller simply expects that when the object is destroyed, all its resources are released.

So release as much as you can. If something goes wrong with one of them, keep going, because there’s still other stuff to clean up.

Related: Never throw an exception from a destructor.

Bonus chatter: Yes, I know that you can avoid this problem by wrapping the Doodad and Gadget handles inside a class which disconnects/destroys on destruction. That’s not my point.

Topics
Code

Author

Raymond has been involved in the evolution of Windows for more than 30 years. In 2003, he began a Web site known as The Old New Thing which has grown in popularity far beyond his wildest imagination, a development which still gives him the heebie-jeebies. The Web site spawned a book, coincidentally also titled The Old New Thing (Addison Wesley 2007). He occasionally appears on the Windows Dev Docs Twitter account to tell stories which convey no useful information.

0 comments

Discussion are closed.