As promised, my least customer impactful bug ever.
VBScript version 1.0 was written by one of the VB compiler devs, one of those uber-productive developers who implements compilers on the weekend for fun. It was implemented and tested extremely rapidly, and as a result, a few of the less important methods in the VB runtime were not ported to the VBScript runtime. My first task as a full timer at Microsoft was to add the missing functions to VBScript 2.0.
On August 5th, 1996, I implemented
LoadPicture, which, as you might imagine, extracts a picture from a storage. Here’s a scrap of the code I wrote:
IDispatch * pdisp; // [... open storage and stream ...] hresult = OleLoadPicture( pstream, 0, TRUE, IID_IPicture, (void **)&pdisp );
Did I mention that I’d been writing COM code for all of two weeks at the time?
There’s a boneheaded bug there. I’m asking for an
IPicture* and assigning the result to an
IDispatch* This is going to crash and burn the moment anyone tries to call any method on that picture object because the vtable is going to be completely horked. I’ve violated one of the Fundamental Rules of COM. The fact that this bad code shipped to customers indicates that:
- Most seriously, I did not adequately test it before I checked it in
- my mentors did not adequately review the code
- the test team did not adequately test it before it shipped to customers
Those are all bad, and I am happy to say that today we are much, much more hard-core about peer-reviewing, self-testing, buddy-testing, and tester-testing code before it goes to customers than we were seven years ago.
But there is a silver lining of a sort — obviously no one at Microsoft bothered to run this code after I wrote it. But neither did any users. We didn’t get a bug report on this thing until February of 1998. This thing was in the wild for almost a year and a half before someone noticed that it was completely, utterly broken! No one really cared.
The next day the name plate on my office door said
LoadPicture Lippert. Ha ha ha, very funny guys.
A few germane questions from the comments:
Is there any use for that thing?
You can’t use it in a web page that loads from the internet for security reasons. HTML-based applications that run locally have better mechanisms for loading pictures. There’s no reason to use it from Windows Script Host or Active Server Pages. The only real use is when you have a trusted script and it is scripting an ActiveX control that can have pictures added to it dynamically.
It’s ironic that this problem would have been avoided had you used your hated smart pointers. Do you know that there are no other places in the script engines where a
CoCreateInstancecreates an interface and the target pointer does not match?
There is exactly one place in the original script engine code calls
CoCreateInstance and it gets it correct; the script engine would crash immediately if that weren’t the case.
Sure, smart pointers would have saved me in this case, but a technique that detects a problem at compile time that would have been caught immediately the first time I ran the code is not really that interesting! The problem here is that I was so unprofessional as to not even run the code I’d just written, and to not show it to anyone who would have immediately realized that it looked wrong.
My distaste for smart pointers is because the bugs that smart pointers cause are deep, insidious, hard to debug, and require detailed knowledge of both the underlying COM concepts that they are abstracting AND detailed knowledge of the smart pointer implementation.
Like I said, I’m not smart enough to use smart pointers.
IUnknown::Release I understand, and when I screw up some refcounting, I know how to fix it quickly. Smart pointers work great most of the time, and when they fail, it takes me all day to track down the problem.