| Date: | October 19, 2007 / year-entry #383 |
| Tags: | code |
| Orig Link: | https://blogs.msdn.microsoft.com/oldnewthing/20071019-00/?p=24733 |
| Comments: | 45 |
| Summary: | Probably the biggest category of problems that can be traced to violating COM single-threaded apartment rules in the shell is using an object from the wrong thread. Of course, nobody admits to doing this up front, They just report that the shell is broken. We can't enumerate the items on the desktop any more. We... |
Probably the biggest category of problems that can be traced to violating COM single-threaded apartment rules in the shell is using an object from the wrong thread. Of course, nobody admits to doing this up front, They just report that the shell is broken.
There isn't enough information to diagnose the problem, and if you just do what they claim doesn't work, you find that it works:
#include <windows.h>
#include <ole2.h>
#include <shlobj.h>
#include <shlwapi.h>
#include <stdio.h>
#include <tchar.h>
INT __cdecl
_tmain(
INT iArgc,
__in_ecount(iArgc) PTSTR ppszArgv[]
)
{
if (SUCCEEDED(CoInitialize(NULL))) {
IShellFolder *psf;
if (SUCCEEDED(SHGetDesktopFolder(&psf))) {
IEnumIDList *peidl;
if (SUCCEEDED(psf->EnumObjects(NULL, SHCONTF_FOLDERS |
SHCONTF_NONFOLDERS, &peidl)) && peidl) {
LPITEMIDLIST pidl;
while (peidl->Next(1, &pidl, NULL) == S_OK) {
STRRET str;
if (SUCCEEDED(psf->GetDisplayNameOf(pidl,
SHGDN_NORMAL, &str))) {
TCHAR sz[MAX_PATH];
if (SUCCEEDED(StrRetToBuf(&str, pidl, sz, MAX_PATH))) {
_tprintf(TEXT("%s\n"), sz);
}
}
ILFree(pidl);
}
}
psf->Release();
}
CoUninitialize();
}
return 0;
}
When given this simple program that does what they claim doesn't work,
the customer explained that they cache the desktop folder.
It works for a while, and then stops working.
The code is complicated, so they haven't been able to isolate the problem yet.
They did find that if they didn't cache the pointer
and just called
I never got a confirmation,
but I'm pretty sure that they are violating COM apartment threading
model rules and obtaining the desktop folder obtained on one thread,
then using it on another.
Apartment model rules specify that you must use an object on the same
thread that created it.
If you want to use it on another thread,
you have to use a helper function like
Moral of the story: Stick to the rules for COM objects. If you don't, you may get away with it for a little while, but someday your sins may catch up to you. |
Comments (45)
Comments are closed. |
I just love it when such a simple COM example needs 6 levels of indentation :)
> It’s the error checking that causes the indentation, not COM.
The question is whether all these opportunities for failure are really necessary. For example, StrRetToBuf can probably be designed in a way that it will not fail if it is passed a valid result from a successful call to GetDisplayNameOf. In that case, the code could become simpler and there would be no need to include a code path that is probably never tested anyway.
Complaining about indentation? Sheesh. Have you ever heard of guard clauses?
http://c2.com/cgi/wiki?GuardClause
If more people actually went through the trouble of checking return values like that then the world would be a better place.
Poor Raymond,
if he excludes error checking people complain,
if he includes error checking people complain.
If only Microsoft could publish the source code of the COM runtime, maybe people would finally understand how it works.
If you want to understand something complex, documentation is usually better than source code. Having the source is a bonus when you need to resolve an ambiguity in the documentation but having better documentation is preferable, I think.
There is a lot of documentation on COM. The problem is that people aren’t always reading/finding/understanding/remembering it. I don’t know which of those four things is most to blame but I don’t think any of them would be solved by having the source code behind COM unless the comments in the code are better than what you’d find in a book (I doubt that).
Having said that, I have yet to read a good explanation of COM threading and apartments. They seem to be something that is very difficult to explain, even for experts in the field. (I think Don Box said something like that himself but I may have a faulty memory.) Maybe I just never found the right article or book, though. (I think I understand them now but it’s the result of piecing together a lot of different articles.)
Let’s be honest. Who really understands COM’s apartment model. Really? I’d view those people who admit to not understanding it to being honest, and either new to COM, or experienced enough to admit that it scares them.
I think only Don Box understood it all, and even then, its been long enough that that he’s forgotten the details.
For anyone who does claim to understand COM, ask them what happens when a COM call triggers a C++ exception in the invoked class.
Straight from the COM spec:
<i>But before we get into error handling in COM, we’ll first take a small digression. Many readers might here be wondering about exceptions. How do exceptions relate to interfaces? In short, it is strictly illegal to throw an exception across an interface invocation; all such cross-interface exceptions which are thrown are in fact bugs in the offending interface implementation.</i>
Another milestone!
REDMOND Today Microsoft Corporation open-sourced the code to their veritable COM runtime. This marks another step into the OSS realm by the software giant.
I’ve heard that apartment model restricts the threads that use a particular object – why is this? I can deal with it being so, I just want to know why it was done this way.
Yeah, I was sort of hoping that someone had a link to a discussion of the tradeoffs in a bit more depth. Since we’re on the sibject and all.
"I’m working on a lot of projects that are entirely F/OSS/GNU/OMGLINUX and I really don’t have the time to dig through the 18 layers of source code to figure out a niggling detail. I’d rather that they give a good manual instead."
Or, you could put the manual as part of the comments in the source code. Like in Java.
> SUCCEEDED(CoInitialize(NULL))
GAH holy function calls in macros Batman!
Yes, I know SUCCEEDED only evaluates its argument once… so you got away with it that time.
(I see HRESULT_FROM_WIN32(GetLastError()) a lot too… that works *even though HRESULT_FROM_WIN32 evaluates its argument multiple times* because the return value is consistent over the life of the macro.)
"If only Microsoft could publish the source code of the COM runtime, maybe people would finally understand how it works."
I’m working on a lot of projects that are entirely F/OSS/GNU/OMGLINUX and I really don’t have the time to dig through the 18 layers of source code to figure out a niggling detail. I’d rather that they give a good manual instead. This isn’t a pot shot at Microsoft at all, because they actually have documentation, and while uneven, it usually gets the point across.
Further, everyone has access to the Linux kernel, but that doesn’t mean everyone understands how OSes work. It takes a lot of work and academic study to "get it." So stop pouting about OSS and OMGNU crap.
As for the indentation, I guess error checking is a good thing. I got used to it when I was writing a whole bunch of COM addins and it isn’t that hard. I even *gasp* refactored my code to make it less offensive.
The shell outright refuses to load free-threaded or apartment-neutral components as shell extensions. Just so you know
Isn’t the shell multithreaded compatible?
mt: No, it isn’t. See yesterdays blog entry.
I wasn’t complaining about error checking or about indentation.
I just said that there are too many things which can fail in such a simple code sample.
"I just said that there are too many things which can fail in such a simple code sample."
Well, some of the error checking could be extraneous. Like for example, I don’t know how SHGetDesktopFolder(&psf) could fail, since all it does (assumedly) is copy a pointer stored somewhere in the bowels of the shell into psf. I know that would be "just an implementation detail", but that’s how people think and you better not let that function fail unless you don’t want to avoid keeping programs from crashing.
I guess COM is complex, and you learn about many of the complexities the hard way.
I imagine it’s possible for there to be a program running on a session without a graphic device (e.g. services, though I don’t know if this is actually the case). In which case there would be no desktop.
"Or, you could put the manual as part of the comments in the source code. Like in Java."
Which is fine for function-specific documentation (like ‘what are the arguments to DoStrangeThingWithPointers(..)?’ or ‘What’s the return value for WeirdFunction()?’) – but there’s a nasty chicken and egg problem: how do I find the function I actually need to do a job?
Just look back a few stories to the mouse-button-swapping problem. The programmer had used a function with a nice obvious name, SwapMouseButton(), which was documented as returning the value he needed, with a documented side-effect he reversed straight afterwards if needed. How was he supposed to know the right function was actually GetSystemMetrics(), which has no obvious connection to swapping mouse buttons? Yes, looking at GetSystemMetrics would have told him – but only if he’d known already which function to look up!
OK, in this specific case Peter has now added ‘Community Content’ to the online MSDN page documenting this, to stop anyone else falling into the same trap, but how many more issues like this are lurking out there? How many more programmers fell into this trap in the decade or so between the function being released and Peter putting a warning online? (Just a quick glance across Raymond’s blog finds another example, someone using a global mouse hook to identify idle time when they should apparently use GetLastInputInfo instead.)
It’s a shame – I found a nice trick in Swing with a watchdog thread, which switched on an hourglass every time the event despatch thread went out to lunch for more than a second or so. Of course, Swing was routing everything into a single thread for UI calls behind the scenes, but it was nice and easy to program because that was all taken care of automatically. Try the same in SWT, it blows up in your face; in Win32…?
Maurits: HRESULT_FROM_WIN32 is now an inline function
I am talking about all you relentless nitpickers who pounce on Raymond almost every day. I think you guys have inferiority complexes, and taking potshots at one of those rare exceptions to the corporate wall of Microsoft makes you feel important. "Look at me! I am not afraid to stick to to Microsoft!" Pathetic!
"I am talking about all you relentless nitpickers who pounce on Raymond almost every day. I think you guys have inferiority complexes, and taking potshots at one of those rare exceptions to the corporate wall of Microsoft makes you feel important. "Look at me! I am not afraid to stick to to Microsoft!" Pathetic!"
Hell yes. I am amazed every day that Raymond doesn’t, like any normal person would, go insane and kill a bunch of people with a sharp object from the incessant needling these idiots give him.
If I were running this site, I would make a Hall of Shame with a list of every single nitpicking idiot that drops by, along with their e-mail addresses (for those that make their e-mail address available) and links to their blogs (for those that have blogs with comments enabled).
Saturday, October 20, 2007 10:16 PM by Justin Olbrantz (Quantam)
"Hell yes. I am amazed every day that Raymond doesn’t, like any normal person would, go insane and kill a bunch of people with a sharp object from the incessant needling these idiots give him."
So nitpicking on a blog is acceptable grounds for murder to you?
"If you want to understand something complex, documentation is usually better than source code. Having the source is a bonus when you need to resolve an ambiguity in the documentation but having better documentation is preferable, I think."
An ambiguity, or something that just isn’t mentioned. Or is just plain wrong. See, for instance, the documentation for CreateWindowEx ( http://msdn2.microsoft.com/en-us/library/ms632680.aspx ). Now tell me how to use the lpParam parameter when the extended style includes WS_EX_MDICHILD?
Now. Go read the source code for Wine’s version of the function at http://source.winehq.org/source/dlls/user32/win.c#L855 .
You see how it works differently to what the documentation suggests? It’s the same in Windows, too.
This coding style of rigorous error checking also makes it easy to spot interface leaks. Because each Release has its "natural" place. Like the missing peidl->Release() in Raymonds sample.
"Like the missing peidl->Release() in Raymonds sample."
/sarcasm on
Yes, that was really easy to spot.
/sarcasm off
"So nitpicking on a blog is acceptable grounds for murder to you?"
*ambiguous yet unsettling grin*
"So nitpicking on a blog is acceptable grounds for murder to you?"
Of course, what a silly question. How will people learn proper polite behaviour otherwise, seeing as they rarely even read the post?
Admittedly I’d probably just kneecap people instead of murdering them, but I’m a big ol’ softy.
I think, by far the worst problem with COM is that implementation has been refined over time, and wrt threading, too. Initially, people were doing strange faulty things and getting away with them. And when it started biting back, of course they were surprised. It’s an upside-down way to refine software really, it should go from stricter to lax.
In COM, there’s staggering amount of low-level details that many people don’t even see. Like with threading, or memory allocation rules wrt marshaling.
Then, if you used MFC’s ( crap ;-) ) support for IDispatch, it changes greatly your view of interaction with COM, as it wraps all in "programmer-friendly" calls, along with data type changes (e.g. from BSTR to LPCTSTR for a BSTR idl property, VARIANT_BOOL to BOOL), or automatic protection against MFC exception.
Now, throw in use of any C++ library that needs exceptions (look no further than STL, that doesn’t work without exceptions), and you’re in for a lot of pain.
Sorry for ranting. Just wanted a say for a little guy.
[A flat API would have the same amount of indentation. It’s the error checking that causes the indentation, not COM. But you knew that; you’re just being snide. -Raymond]
Is the ‘goto to cleanup’ method considered bad style at Microsoft? It does save you all this indentation.
Would the company that invented BASIC think GOTO statement is a bad programming style?
@Jules: I don’t follow you; the documentation and WINE source seem to be in agreement.
The documentation says that the WS_EX_MDICHILD flag is used to create a MDI child window. Then it says that if you are creating a MDI child window you should pass a pointer to a MDICREATESTRUCT struct. The WINE source seems to …discussion of GNU-protected source code deleted…
@640k:
Microsoft didn’t invent basic or goto.
Goran: COM *did* go from "more restrictive" to "more lax" with time. The first threading model was the STA — about as "strict" as you can get, in my opinion…
Raymond’s just following structured programming practices, in particular single point of exit, and ensuring acquired resources are also freed. Without the use of classes the only alternative is to duplicate code.
But, that’s not the point of Raymond’s post; but as usual the usual group of nickpickers has hijacked the topic…
"Hell yes. I am amazed every day that Raymond doesn’t, like any normal person would, go insane and kill a bunch of people with a sharp object from the incessant needling these idiots give him."
We try to keep Raymond away from sharp objects, although knitting needles seem to calm him.
Dean: I was talking about enforcements on threading rules, not about the growth of threading models***. In that sense, before, if you cheated, you could get away thinking it’s OK to do it or being completely oblivious to the issues. For an outsider and a little guy (me), it’s a change for the worse.
***I think, for all the failures e.g. CORBA had, threading was simpler and more effective. (Well, it was the last time I looked into it). I know goals are not the same as in (D)COM, but still…
@john.
This isn’t the right forum for this,but yes thats what the windows docs say.
But … discussion of GNU-protected source code deleted…
Ok, I see what you’re saying. I traced the function calls in the Wine source and … discussion of GNU-protected source code deleted …
Anyway … it appears you guys are correct. The documentation and actual behavior do not match.
(Nitpick: It’s not GNU-protected code. It’s LGPL-protected code. ;-) )
But yeah, I wondered how long it’d be until that got deleted, knowing what usually happens when stuff like it gets brought up. :-)