The latest build of MFCMAPI was just added to the internal dev tools collection for the next version of Office. This means a lot of the developers and testers are running MFCMAPI against their private, debug builds of Outlook. One of the developers e-mailed me last week to let me know that this build of MFCMAPI was causing debug assertions. They took a look at the stack and told me the function throwing the assertion was MAPIFreeBuffer. They also pointed out where I was making the call.
Horror of horrors! Had I somehow attempted to MAPIFreeBuffer memory that hadn't been allocated via MAPI? Or was I freeing the same memory twice? After all the preaching I do to my customers over the importance of good MAPI memory management had I committed one of the sins I caution against?
Well, yes and no. I had indeed made an error, but it wasn't one I had seen before.
MFCMAPI displays a lot of data in list boxes. A row in a list box has a single data pointer. I have a structure that contains pointers to various buffers to hold things like Entry IDs. To simplify cleanup, I allocate the main structure with MAPIAllocateBuffer, then I allocate buffers that hang off of this structure with MAPIAllocateMore. That way, a single call to MAPIFreeBuffer can free the structure and all of the extra buffers. This makes my code very clean.
Sometimes I need to wipe out one of these buffers and replace the contents. This is where I got into trouble. I didn't want to leak any memory, so I called MAPIFreeBuffer on the buffer, set the pointer to NULL, then allocated a new buffer with MAPIAllocateMore. That call to MAPIFreeBuffer is the one that caused the assertion. You cannot free memory allocated with MAPIAllocateMore by calling MAPIFreeBuffer on it. The only way to free that memory is to call MAPIFreeBuffer on the 'parent' memory which you indicated in the call to MAPIAllocateMore.
So, lesson learned. Here are some other common MAPI memory leaks I see in customer's code (heck, I think I've seen every one of them in our code at one point or another):
- Freeing LPSRowSet and LPAdrList pointers with MAPIFreeBuffer: Big mistake, big leak. These special array based structures are not built with MAPIAllocateMore. Each entry in the list is allocated with it's own call to MAPIAllocateBuffer. Special structures need special cleanup functions, FreeProws and FreePadrlist. These functions walk the arrays and call MAPIFreeBuffer on each entry before calling MAPIFreeBuffer on the array itself. (A neat side effect of this is you can 'rip' an entry out of an array and stuff it somewhere else without having to copy anything. Just NULL out the array entry and it's yours. Make sure you free it when you're done!)
- Calling MAPIFreeBuffer on MAPI objects: You wouldn't believe how many times I've seen this.
- Calling UlRelease on MAPI structures: Or this.
- Not calling Release when you're done with MAPI objects: Developers come up with all sorts of crazy reasons to justify this practice. My favorite was a developer who was deleting messages with IMAPIFolder::DeleteMessage. He had an IMessage object pointing at the message he was deleting and wouldn't call Release on it because 'the message had been deleted'. He was suprised to when he started getting MAPI_E_CALL_FAILED when calling OpenEntry after his app had been running a long time.
So how did I fix my bug? The answer was quite simple - don't call MAPIFreeBuffer here! The memory I had allocated will be freed when the parent memory is freed, regardless of whether or not I still have a pointer to it. Since this scenario was rare, I can afford the memory hit of having a few extra buffers allocated for the lifetime of the structure.
[Comments for this post have been closed]