Share via


What's wrong with this code, part 18, plus a bonus bad code

So the last "What's wrong with this code" article was dead easy, I knew it was likely that people would find it such.

 

patria found the answer on the 4th comment, and I think that Mike Dimmick put it best:

Well, if you're going to use the Resource Acquisition Is Initialization idiom, use it consistently:
 

CComPtr (an autoptr for COM objects that auto-addref's and releases the object) uses RAII, but the code didn't consistently use RAII  - instead it pretended that it wasn't using RAII.  Since CComPtr never throws, it's easy to treat it as a super pointer, but Mike's right - you have to be careful about lifetime issues, and that's exactly what went wrong in this example.

The problem is that when you call CoUninitialize, you need to ensure that you've released all references to any COM objects you might hold, if you don't, the DLL that hosts the COM objects is almost certainly going to have been uninitialized.

So let's present a "fixed" version of the code, using Mike's example of adding an RAII style object to work around the lifetime issue:

class

CCoInitializer
{
public:
CCoInitializer( DWORD dwCoInit )
{
HRESULT hr;
hr = CoInitializeEx( NULL, dwCoInit );
        if (FAILED(hr))
{
            throw hr;
}
}

~CCoInitializer()

throw()
{
CoUninitialize();
}
};

int

_tmain(int argc, _TCHAR* argv[])
{
HRESULT hr;
CCoInitializer coInitializer(COINIT_APARTMENTTHREADED);
CComPtr<IHTMLDocument2> document;
CComPtr<IHTMLElementCollection> images;
CComPtr<IDispatch> image;
LONG imageCount;

hr = document.CoCreateInstance(CLSID_HTMLDocument);
    if (FAILED(hr))
{
exit(hr);
}
:
:
hr = document->get_images(&images);
    if (FAILED(hr))
{
exit(hr);
}
hr = images->get_length(&imageCount);
    if (FAILED(hr))
{
exit(hr);
}
    for (LONG i = 0 ; i < imageCount ; i += 1)
{
hr = images->item(CComVariant(), CComVariant(i), &image);
        if (FAILED(hr))
{
exit(hr);
}
}
    return 0;
}

While I was fixing the code, I added a bit of additional stuff.  Unfortunately, the new code introduced yet another bug.

Btw, for those playing along at home, I know that this doesn't actually work, code to load up the HTML document is in the omitted section :).

In addition, the absence of code to check for the coInitializer object throwing is NOT a bug.  There's no way of recovering from this exception, and the exception handling paradigm states that if you don't know how to handle an exception, you let your caller handle it.