{"id":9923,"date":"2011-08-11T07:00:00","date_gmt":"2011-08-11T07:00:00","guid":{"rendered":"https:\/\/blogs.msdn.microsoft.com\/oldnewthing\/2011\/08\/11\/the-ways-people-mess-up-iunknownqueryinterface-episode-4\/"},"modified":"2011-08-11T07:00:00","modified_gmt":"2011-08-11T07:00:00","slug":"the-ways-people-mess-up-iunknownqueryinterface-episode-4","status":"publish","type":"post","link":"https:\/\/devblogs.microsoft.com\/oldnewthing\/20110811-00\/?p=9923","title":{"rendered":"The ways people mess up IUnknown::QueryInterface, episode 4"},"content":{"rendered":"

\nOne of the rules for IUnknown::Query­Interface<\/code> is so\nobvious that nobody even bothers to state it explicitly as a rule:\n“If somebody asks you for an interface,\nand you return S_OK<\/code>,\nthen the pointer you return must point to the interface the caller\nrequested.”\n(This feels like the software version of\n\ndumb warning labels<\/a>.)\n<\/p>\n

\nDuring compatibility testing for Windows Vista,\nwe found a shell extension that behaved rather strangely.\nEventually, the problem was traced to\na broken IUnknown::Query­Interface<\/code>\nimplementation which depended subtly on the order in which\ninterfaces were queried.\n<\/p>\n

\nThe shell asked for the IExtract­IconA<\/code> and\nIExtract­IconW<\/code> interfaces in the following\norder:\n<\/p>\n

\n\/\/ not the actual code but it gets the point across\nIExtractIconA *pxia;\nIExtractIconW *pxiw;\npunk->QueryInterface(IID_IExtractIconA, &pxia);\npunk->QueryInterface(IID_IExtractIconW, &pxiw);\n<\/pre>\n
\nOne particular shell extension would return the same pointer to\nboth queries;\ni.e.<\/i>,\nafter the above code executed,\npxia == pxiw<\/code> even though neither interface\nderived from the other.\nThe two interfaces are not binary-compatible,\nbecause IExtract­IconA::Get­Icon­Location<\/code>\noperates on ANSI strings, whereas\nIExtract­IconW::Get­Icon­Location<\/code> operates\non Unicode strings.\n<\/p>\n
\nThe shell called pxiw->Get­Icon­Location<\/code>,\nbut the object interpreted the szIcon­File<\/code> as\nan ANSI string buffer; as a result, when the shell went to look at it,\nit saw gibberish.\n<\/p>\n
\nFurther experimentation revealed that if the order of the two\nQuery­Interface<\/code> calls were reversed,\nthen pxiw->Get­Icon­Location<\/code> worked as expected.\nIn other words,\nthe first interface you requested “locked” the object into that\ninterface, and asking for any other interface just returned\na pointer to the locked interface.\nThis struck me as very odd; coding up the object this way\nseems to be harder<\/i> than doing it the right way!\n<\/p>\n
\n\/\/ this code is wrong - see discussion above\nclass CShellExtension : public IExtractIcon\n{\n enum { MODE_UNKNOWN, MODE_ANSI, MODE_UNICODE };\n  HRESULT CShellExtension::QueryInterface(REFIID riid, void **ppv)\n  {\n   *ppv = NULL;\n   if (riid == IID_IUnknown) *ppv = this;\n   else if (riid == IID_IExtractIconA) {\n    if (m_mode == MODE_UNKNOWN) m_mode = MODE_ANSI;\n    *ppv = this;\n   } else if (riid == IID_IExtractIconW) {\n    if (m_mode == MODE_UNKNOWN) m_mode = MODE_UNICODE;\n    *ppv = this;\n   }\n   if (*ppv) AddRef();\n   return *ppv ? S_OK : E_NOINTERFACE;\n  }\n  ... AddRef \/ Release ...\n  HRESULT GetIconLocation(UINT uFlags, LPTSTR szIconFile, UINT cchMax,\n                          int *piIndex, UINT *pwFlags)\n  {\n   if (m_mode == MODE_ANSI) lstrcpynA((char*)szIconFile, \"foo\", cchMax);\n   else lstrcpynW((WCHAR*)szIconFile, L\"foo\", cchMax);\n   ...\n  }\n  ...\n}<\/i>\n<\/pre>\n
\nInstead of implementing both IExtract­IconA<\/code> and\nIExtract­IconW<\/code>, my guess is that they implemented\njust one of the interfaces and made it alter its behavior based\non which interface it thinks it needs to pretend to be.\nIt never occurred to them that the single interface might need\nto pretend to be two different things at the same time.\n<\/p>\n
\nThe right way of supporting two interfaces is to actually implement\ntwo interfaces and not write a single morphing interface.\n<\/p>\n
\nclass CShellExtension : public IExtractIconA, public IExtractIconW\n{\n  HRESULT CShellExtension::QueryInterface(REFIID riid, void **ppv)\n  {\n   *ppv = NULL;\n   if (riid == IID_IUnknown ||\n       riid == IID_IExtractIconA) {\n    *ppv = static_cast<IExtractIconA*>(this);\n   } else if (riid == IID_IExtractIconW) {\n    *ppv = static_cast<IExtractIconW*>(this);\n   }\n   if (*ppv) AddRef();\n   return *ppv ? S_OK : E_NOINTERFACE;\n  }\n  ... AddRef \/ Release ...\n  HRESULT GetIconLocation(UINT uFlags, LPSTR szIconFile, UINT cchMax,\n                          int *piIndex, UINT *pwFlags)\n  {\n   lstrcpynA(szIconFile, \"foo\", cchMax);\n   return GetIconLocationCommon(uFlags, piIndex, pwFlags);\n  }\n  HRESULT GetIconLocation(UINT uFlags, LPWSTR szIconFile, UINT cchMax,\n                          int *piIndex, UINT *pwFlags)\n  {\n   lstrcpynW(szIconFile, L\"foo\", cchMax);\n   return GetIconLocationCommon(uFlags, piIndex, pwFlags);\n  }\n  ...\n}<\/i>\n<\/pre>\n
\nWe worked around this in the shell by simply changing the order\nin which we perform the calls to\nIUnknown::Query­Interface<\/code>\nand adding a comment explaining why the order of the calls is important.\n<\/p>\n
\n(This is another example of how the cost of a compatibility fix\n\nis small potatoes<\/a>.\n\nThe cost of deciding whether or not to apply the fix far exceeds\nthe cost of just doing it for everybody<\/a>.)\n<\/p>\n
\nA different shell extension had a compatibility problem that also was\ntraced back to a dependency on the order in which the shell\nasked for interfaces.\nThe shell extension registered as a context menu extension,\nbut when the shell tried to create it, it got E_NO­INTERFACE<\/code>\nback:\n<\/p>\n
\nCoCreateInstance(CLSID_YourAwesomeExtension, NULL,\n                 CLSCTX_INPROC_SERVER, IID_IContextMenu, &pcm);\n\/\/ returns E_NOINTERFACE?\n<\/pre>\n
\nThis was kind of bizarre.\nI mean, the shell extension went to the effort of registering itself\nas a context menu extension,\nbut when the shell said,\n“Okay, it’s show time, let’s do the context menu dance!”\nit replied,\n“Sorry, I don’t do that.”\n<\/p>\n
\nThe vendor explained that the shell extension relies\non the order in which the shell asked for interfaces.\nThe shell used to create and initialize the extension like this:\n<\/p>\n
\n\/\/ error checking and other random bookkeeping removed\nIShellExtInit *psei;\nIContextMenu *pcm;\nCoCreateInstance(CLSID_YourAwesomeExtension, NULL,\n                 CLSCTX_INPROC_SERVER, IID_IShellExtInit, &psei);\npsei->Initialize(...);\npsei->QueryInterface(IID_IContextMenu, &pcm);\npsei->Release();\n\/\/ use pcm\n<\/pre>\n
\nWe changed the order in a manner that you would think should be equivalent:\n<\/p>\n
\nCoCreateInstance(CLSID_YourAwesomeExtension, NULL,\n                 CLSCTX_INPROC_SERVER, IID_IContextMenu, &pcm);\npcm->QueryInterface(IID_IShellExtInit, &psei);\npsei->Initialize(...);\npsei->Release();\n<\/pre>\n
\n(Of course, it’s not written in so many words in the code;\nthe various parts are spread out over different components and\nhelper functions,\nbut this is the sequence of calls the shell extension sees.)\n<\/p>\n
\nThe vendor explained that their shell extension will not respond\nto any shell extension interfaces (aside from\nIShell­Ext­Init<\/code>)\nuntil it has been initialized,\nbecause it is at that point that they decide which extensions\nthey want to support.\nUnfortunately, this violates the first of the\n\nfour explicit rules for IUnknown::Query­Interface<\/code><\/a>,\nnamely that the set of interfaces must be static.\n(It’s okay to have an object expose different interfaces\nconditionally, as long as it understands that once it says yes or no\nto a particular interface,\nit is committed to answering the same way for the lifetime of the object.)<\/p>\n","protected":false},"excerpt":{"rendered":"
One of the rules for IUnknown::Query­Interface is so obvious that nobody even bothers to state it explicitly as a rule: “If somebody asks you for an interface, and you return S_OK, then the pointer you return must point to the interface the caller requested.” (This feels like the software version of dumb warning labels.) During […]<\/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-9923","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
One of the rules for IUnknown::Query­Interface is so obvious that nobody even bothers to state it explicitly as a rule: “If somebody asks you for an interface, and you return S_OK, then the pointer you return must point to the interface the caller requested.” (This feels like the software version of dumb warning labels.) During […]<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/9923","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=9923"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/9923\/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=9923"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=9923"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=9923"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}