Joel’s law of leaky abstractions rears its ugly head once more. I try to never use smart pointers because… I’m not smart enough.
COM programmers are of course intimately familiar with
Release. The designers of COM decided to use reference counting as the mechanism for implementing storage management in COM, and decided to put the burden of implementing and calling these methods upon the users.
It is very natural when using a language like C++ to say that perhaps we can encapsulate these semantics into an object, and let the C++ compiler worry about calling the
Releases in the appropriate constructors, copy constructors and destructors. It is very natural and very tempting, and I avoid template libraries that do so like the plague.
Everything usually works fine until there is some weird, unforeseen interaction between the various parts. Let me give you an example: Suppose you have the following situation: you have a C++ template library map mapping
IFoo pointers onto
IBar pointers, where the pointers to the
IBar are in smart pointers. You want the map to take ownership of the pointers. Does this code look correct?
map[srpFoo.Disown()] = srpBar.Disown();
It sure looks correct, doesn’t it?
Look again. Is there a memory leak there?
I found code like this in a library I was maintaining once, and since I had never used smart pointer templates before, I decided to look at what exactly this was doing at the assembly level. A glance at the generated assembly shows that the order of operations is:
srpFoo.Disown() to get the
srpBar.Disown() to get the
operator passing in the
IFoo*, returning an
4) do the assignment of the
IBar* to the address returned in (3).
So where is the leak? This library had C++ exception handling for out-of-memory exceptions turned on. If the
operator throws an out-of-memory exception then the not-smart
IBar* presently on the argument stack are both going to leak.
The correct code is to copy the pointers before you disown them:
map[srpFoo] = srpBar;
Before the day that I found this I had never been in a situation before where I had to think about the C++ order of operations for assigning and subscripting in order to get the error handling right! The fact that you have to know these picky details about C++ operator semantics in order to get the error handling right is an indication that people are going to get it wrong.
Let me give you another example. One day I was adding a feature to this same library, and I noticed that I had caused a memory leak. Clearly I must have forgotten to call
Release somewhere, or perhaps some smart pointer code was screwed up. I figured I’d just put a breakpoint on the
Release for the object, and figure out who was calling the extra
Here — and I am not making this up! — is the call stack at the point of the first AddRef:
ATL::CComPolyObject<CLayMgr>::AddRef ATL::CComObjectRootBase::OuterAddRef ATL::CComContainedObject<CLayMgr>::AddRef ATL::AtlInternalQueryInterface ATL::CComObjectRootBase::InternalQueryInterface CLayMgr::_InternalQueryInterface ATL::CComPolyObject<CLayMgr>::QueryInterface ATL::CComObjectRootBase::OuterQueryInterface ATL::CComContainedObject<CLayMgr>::QueryInterface CDDS::FinalConstruct ATL::CComPolyObject<CDDS>::FinalConstruct ATL::CComCreator<ATL::CComPolyObject<CDDS>>::CreateInstance CTryAssertComCreator<ATL::CComPolyObject<CDDS>>::CreateInstance ATL::CComClassFactory2<CDLic>::CreateInstance(ATL::CComClassFactory2 CTryAssertClassFactory2<CDLic>::CreateInstance(CTryAssertClassFactory2<CDLic>
Maybe you ATL programmers out there are smarter than me. In fact, I am almost certain you are, because I have not the faintest idea what the differences between a
CComObjectRootBase and a
CComPolyObject are! It took me hours to debug this memory leak. So much for smart pointers saving time!
I am too dumb to understand that stuff, so when I write COM code, my implementation of
AddRef is one line long, not hundreds of lines of dense macro-ridden, templatized cruft winding its way through half a dozen wrapper classes.
Some reader responses:
Do you really think [the original example] is a normal way of using smart pointers? It is not.
I have no idea if it is “normal” or not, but it is how I found the code that I was reading, and there was plenty more code like it. Saying “you’re doing it wrong” doesn’t help. If the easy, obvious way to do a thing, and the correct, safe way to do a thing are different, then we have a problem.
You don’t have to use or understand things like
CComObjectRootBaseif all you want is a smart pointer.
Well, first off, I don’t want smart pointers. Second, again, this was not a hole of my own digging here. I was asked to help out with an existing codebase that used smart pointers, and debugging memory leaks in it really slowed me down.
The real problem is that exceptions were bolted on to C++ late in the process.
I agree that exceptions are a poor fit for non-GC’d languages, but again, that’s the world we live in, and we should adapt our tools to reality, rather than building tools that make real-world scenarios more difficult to analyze.
Using smart pointers is almost always a better idea than trying to manage the refcounts “by hand”.
I thoroughly disagree. I’ve debugged ref count bugs caused by me forgetting an
AddRef, and I’ve debugged ref count bugs like the one above with the fifteen-deep call stack of meaningless (to me) gibberish, and the former are a whole lot easier to track down.
When I write COM code it tends to emphasize the error paths and cleanup code rather than the program logic, and that’s unfortunate. But given that the error paths and cleanup code are where most of the bugs are, I want them to be as explicit as possible. Those are the program logic.
I understand the urge to abstract them away into classes that take care of the details for you. But in my experience, the abstraction is both complex and leaky. The abstraction is easy to use for simple cases, and thereby affords creation of much more subtle, hard-to-debug bugs.
My point, simply, is that
QueryInterface are a pain in the rear to use, but they are easy to understand. Smart pointers are easy to use and hard to understand, and I prefer to understand the code I write.
So basically you are saying smart pointers suck, because you don’t know how to use them properly?
No, I’m saying they suck because they are easy to accidentally use improperly, and because they slow down my ability to understand and debug a buggy codebase.
An abstraction is supposed to relieve you of the burden of understanding the abstracted thing, but smart pointers do a poor job of that. In order to use them correctly, you have to understand exactly what they’re doing to ensure that you don’t use smart pointers to violate the very rules they are designed to abstract away.
That’s a lousy abstraction. It is easier for me to learn the rules of COM and apply them than to learn the rules of COM and learn the entire smart pointer framework.
The .NET framework does a much better job of abstracting away the details of how the underlying system works, because it was designed to do that from day one. Smart pointers are a kludge written post hoc.
You have bad smart pointers! Don’t blame a concept because of a lame implementation!
Again, I’m not the one choosing what implementation of smart pointers I get to use. Maybe there are smart pointer libraries that do not have these problems, but I’m betting they just have different problems.
This code has exactly the same problem, and no smart pointers:
map[new Foo()] = new Bar();
Sure, and I can look at that code and immediately know that it is wrong without having to understand the semantics of “disown” operations. The only thing I need to know to deduce the wrongness of that code is the basic rules of C++.