IUnknown::QueryInterface<\/code><\/a>:\n<\/p>\n\nCComQIPtr<ISomeInterface> spsi(punkObj);\n...\n\/\/ spsi object goes out of scope\n<\/pre>\n\nIf the IUnknown::QueryInterface<\/code> method puts a\nnon-NULL<\/code> value in spsi<\/code> on failure,\nthen when spsi<\/code> is destructed, it’s going\nto call IUnknown::Release<\/code> on itself,\nand something bad happens.\nIf you’re lucky, you will crash because the thing lying around in\nspsi<\/code> was a garbage pointer.\nBut if you’re not lucky, the thing lying around in spsi<\/code>\nmight be a pointer to a COM object:\n<\/p>\n\n\/\/ wrong!\nHRESULT CObject::QueryInterface(REFIID riid, void **ppvObj)\n{\n *ppvObj = this; \/\/ assume success since it almost always succeeds\n if (riid == IID_IUnknown || riid == IID_IOtherInterface) {\n AddRef();\n return S_OK;\n }\n \/\/ forgot to set *ppvObj = NULL\n return E_NOINTERFACE;\n}<\/i>\n<\/pre>\n\nNotice that this code optimistically sets the output pointer to\nitself, but if the interface is not supported, it changes its mind\nand returns E_NOINTERFACE<\/code> without setting the\noutput pointer to NULL<\/code><\/i>.\nNow you have an elusive reference counting bug,\nbecause the destruction of spsi<\/code> will call\nCObject::Release<\/code>,\nwhich will manifest itself by CObject<\/code> object\nbeing destroyed prematurely because you just over-released the object.\nIf you’re lucky, that’ll happen relative soon;\nif you’re not lucky, it won’t manifest itself for another half hour.\n<\/p>\n\nOkay, sure, maybe this is too obvious a mistake for\nCObject::QueryInterface<\/code>, but any method that\nhas an output parameter can suffer from this error,\nand in those cases it might not be quite so obvious:\n<\/p>\n\n\/\/ wrong!\nHRESULT CreateSurface(const SURFACEDESC *psd,\n ISurface **ppsf)\n{\n *ppsf = new(nothrow) CSurface();\n if (!*ppsf) return E_OUTOFMEMORY;\n HRESULT hr = (*ppsf)->Initialize(psd);\n if (SUCCEEDED(hr)) return S_OK;\n (*ppsf)->Release(); \/\/ throw it away\n \/\/ forgot to set *ppsf = NULL\n return hr;\n}<\/i>\n<\/pre>\n\nThis imaginary function\ntakes a surface description and tries to create a surface\nthat matches it.\nIt does this by first creating a blank surface,\nand then initializing the surface.\nIf that succeeds, then we succeed;\notherwise, we clean up the incomplete surface and fail.\n<\/p>\n
\nExcept that we forgot to set *ppsf = NULL<\/code>\nin our failure path.\nIf initialization fails, then we destroy the surface,\nand the pointer returned to the caller points to the\nsurface that we abandoned.\nBut the caller shouldn’t be looking at that pointer because\nthe function failed, right?\n<\/p>\n\nWell, unless the caller called you like this:\n<\/p>\n
\nCComPtr<ISurface> spsf;\nif (SUCCEEDED(CreateSurface(psd, &spsf))) {\n ...\n}\n<\/pre>\n\nIf the surface fails to initialize, then spsf<\/code>\ncontains a pointer to a surface that has already been deleted.\nWhen the spsf<\/code> is destructed, it’s going to call\nISurface::Release<\/code> on some point that is no longer\nvalid, and bad things are going to happen.\nThis can get particularly insidious when spsf<\/code> is\nnot a simple local variable but rather a member of\nclass which itself doesn’t get destroyed for a long time.\nThe bad pointer sits in m_spsf<\/code> like a time bomb.\n<\/p>\n\nAlthough all the examples I gave here involve COM interface pointers,\nthe rule applies to all output parameters.\n<\/p>\n
\nCComBSTR bs;\nif (SUCCEEDED(GetName(&bs)) { ... }\n\/\/ -or-\nCComVariant var;\nif (SUCCEEDED(GetName(&var)) { ... }\n<\/pre>\n\nIn the first case, the the GetName<\/code> method had\nbetter not leave garbage in the output BSTR<\/code> on failure,\nbecause the CComBSTR<\/code>\nis going to SysFreeString<\/code> in its destructor.\nSimilarly in the second case with CComVariant<\/code>\nand VariantClear<\/code>.\n<\/p>\n\nSo remember, if your function doesn’t want to return a value\nin an output pointer, you still have to return something in it.<\/p>\n","protected":false},"excerpt":{"rendered":"
We saw last time that the COM rules for output pointers are that they must be initialized on return from a function, even if the function fails. The COM marshaller relies on this behavior, but then again, so do you; you just don’t realize it yet. If you use a smart pointer library (be it […]<\/p>\n","protected":false},"author":1069,"featured_media":111744,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[1],"tags":[25],"class_list":["post-15443","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
We saw last time that the COM rules for output pointers are that they must be initialized on return from a function, even if the function fails. The COM marshaller relies on this behavior, but then again, so do you; you just don’t realize it yet. If you use a smart pointer library (be it […]<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/15443","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/users\/1069"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/comments?post=15443"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/15443\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media\/111744"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media?parent=15443"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=15443"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=15443"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}