| Date: | September 7, 2006 / year-entry #304 |
| Tags: | code |
| Orig Link: | https://blogs.msdn.microsoft.com/oldnewthing/20060907-04/?p=29823 |
| Comments: | 31 |
| Summary: | Everybody should know by now that you have to free memory using the same allocator that you used to allocate the memory. If you allocate with new[] then you have to free with delete[]; if you allocate with LocalAlloc then you have to free with LocalFree. Once you've internalized this rule, you can use it... |
Everybody should know by now that you have to free memory using the same allocator that you used to allocate the memory. If you allocate with Once you've internalized this rule, you can use it to draw other logical conclusions. Consider:
Well, there are two candidates for this responsibility, either the The only remaining candidate is the caller of the |
Comments (31)
Comments are closed. |
And in older windows, not only did the right function have to free the memory, but it had to be called from the same DLL that allocated the memory (because each DLL had its own heap).
Thankfully those days are over.
Also… remember that if a DLL has the C run-time statically linked, that DLL can only deallocate memory that was allocated from within it (and memory allocated from within the DLL cannot be deallocated by the main module or other DLLs). A DLL with the CRT statically linked has its own copy of the CRT (of course), so it has its own heap.
(When I say allocate/deallocate here, I’m referring to new/delete and malloc/free.)
I ran into this with FindMimeFromData:
http://msdn.microsoft.com/workshop/networking/moniker/reference/functions/findmimefromdata.asp
‘Who’ is easy. By the ‘FindMimeFromData isn’t psychic’ principle, I must free the memory myself. The next question is ‘how’. The shell allocator? LocalFree? GlobalFree? delete? free? Sounds like it is our turn to be psychic.
I’m sure there is a general principle for which components use which method. I haven’t found it, though. (Obviously delete and free are wrong, and GlobalFree is a pretty bad guess too…)
Actually, I had thought ppwzMimeOut…
LPWSTR* eh?
I would have guessed SysFreeString() (Assuming it’s a BSTR)…
But you are right, it isn’t defined…
Reminds me of the early C++ strstreams, that allowed you access to a character pointer, but you had to then free the memory it had allocated.
Not that you knew how it had been allocated.
Oh, OK, I see.
Yeah, most MSDN reference pages for functions that allocate memory on the caller’s behalf have a section on how to free that memory (e.g., the reference for InitializeSecurityContext says that when you pass it a flag of ISC_REQ_ALLOCATE_MEMORY, you have to call FreeSecurityContext to deallocate the memory that it allocated).
Googling for FindMimeFromData reveals this pinvoke.net page:
http://www.pinvoke.net/default.aspx/urlmon.FindMimeFromData
which seems to imply you need to use Marshal.FreeCoTaskMem — see the “alternative managed APIs” section, it’s not in the main reference. (I’m not sure which deallocator that uses, though it’s probably documented somewhere also. I’d suspect it’s something to do with COM though.)
Anyway, yes, it should be in the documentation (and I’ve submitted a request that it be added, using the “contact” link at the bottom of that page; we’ll see what happens with that).
CoTaskMemFree? It’s what you have to use for StringFromCLSID and other such APIs.
CoTaskMemFree sounds like it should work, assuming that both (1) the pinvoke.net information is correct, and (2) that’s what FreeCoTaskMem maps to. (I don’t see why it wouldn’t map to that though; the names are uncannily similar).
Yup, it does wrap CoTaskMemFree:
http://msdn2.microsoft.com/en-us/library/system.runtime.interopservices.marshal.freecotaskmem.aspx
"FreeCoTaskMem exposes the CoTaskMemFree COM API function, which frees all bytes so that you can no longer use the memory pointed to by the ptr parameter. For additional information about CoTaskMemFree, see the MSDN library."
Still, I find it interesting that we have the BSTR functions, but a bunch of the Win32 system calls that return Unicode strings use CoTaskAlloc…
I guess the win systems team hates BSTRs as most of us app developers…
Just so Raymond doesn’t have to say it :-)
In the past year I’ve sent dozens of documentation corrections to Microsoft; overwhelmingly I got a quick and polite response (even when I’ve been wrong). Even better, most answers say that the problem will be fixed.
Everyone: whenever you see issues about who frees memory, but sure to tell Microsoft about it.
Try calling FindMimeFromData twice with the same parameters. I’m guessing that the two values for ppwzMimeOut will point to the same memory, which means it’s a deliberate, but limited, memory leak.
In some cases, the function description tells you what to use. Consider (practically) any function that returns a BSTR – It allocates the BSTR, but the caller is the one who needs to free it.
In this case, the common way of doing this is to SysFreeString() the bstr.
And that’s why documentation of an API is important.
ajb: What memory? Presumably you mean the memory pointed to by the pBuffer parameter, right?
Then, the answer to your "how" question is compeletely dependent on how you got the value that you passed for pBuffer: If you called new[] to get it, then delete[] *is* appropriate. If you used the shell allocator, then use the shell allocator’s free function. If you used LocalAlloc, then use LocalFree.
However, if you got that pointer from other code (so you don’t know how to free it), then it’s the resposibility of that other code to free it, not you.
Unless you meant one of the other parameters…
"FreeCoTaskMem exposes the CoTaskMemFree COM API function, which frees all bytes so that you can no longer use the memory pointed to by the ptr parameter."
That’s quite funny. This implies that the *purpose* of FreeCoTaskMem is to stop you using the memory, rather than to avoid a resource leak or reduce your process’s heap size… Surely if you didn’t want to use the memory any more, you would simply not use the memory any more?
Oh, I agree, except this function doesn’t really return a BSTR*, but a LPWSTR*.
And yes I know BSTR is typedef’ed as LPWSTR, but it ISN’T the same as an array of wide characters.
(A BSTR is a pointer to an array of wchar’s, EXCEPT that the 4-bytes behind the pointer is the length of the string – I had to learn this the hard way when passing L"foo" string literals didn’t marshal across to an out-of-proc COM component).
My point is that it probably the wrong thing to treat an LPWSTR as a BSTR with regards to using SysFreeString() – That array of wide chars may not have the 4-byte length indicator behind it.
Xan: It’s perfectly safe, including thread-safe, to return the same pointer twice, when it’s the same text value you want returned, as long as the caller doesn’t modify the string.
LEJ: The Windows developers can’t hate the counted Unicode strings all that much, considering that’s the API which underpins the whole thing, from filenames on NTFS to Registry entries :-)
It’s discussions like this that really make you start to see the value of garbage collection in .NET.
to Anders Munch:
If it returns the same pointer for (and only for) the same input (and output), it’s thread safe but then clearly you cannot free the memory with any method (unless an ad-hoc one is provided) because the function is clearly referencing some data structure to associate input<->ptr and freeing the ptr memory would corrupt future calls with the same input.
If, instead, it returns the same ptr for different inputs we have two cases. If the ptr is in thread dependent storage (for example in the TLS) the it’s thread safe but prone to the double call problem seen in my previous message (an example of this is strtok). If the ptr is always absolutely the same, then it’s critically thread unsafe because not only the function call has to be enclosed in a critical section (or whatever synch method) but even all the code parsing its output.
[I just did a quick check. FindMimeFromData appears to use an internal allocator that is not accessible to callers. It looks to me like it’s impossible to free the data! Oops. -Raymond]
Oops indeed. Well it looks like Wine has it wrong then because their implementation of this function uses CoTaskMemAlloc.
to Andy C:
How so? Garbage collection is not helpfull to this situation. A single environment wide allocator on the other hand is.
Chris: Full system-wide GC would help because people wouldn’t have to worry about freeing *anything*, including the various types of strings. I suspect that’s what Andy meant, anyway.
Anders: But if that’s what’s going on, it should also be documented that "the caller must not modify this string".
Norman: Well, so far, this particular issue hasn’t resulted in any strange responses like that. I received an email from someone saying they were working to get an answer on how exactly to free that memory. Presumably they will update the docs too, once they find out. (If it’s even possible for the caller to free it; from Raymond’s response, it sounds like it isn’t. Yikes.)
But that’s probably because on the form I filled out, I chose "United States" as the country. Presumably you chose "Japan"; that might be the difference. Not sure why it would matter, though…
BryanK: There are resources other than memory. Most of them are scarcer than memory and have more contention.
Locks. File handles (file handles themselves don’t have much contention, but the underlying files do). Mutexes. etc… All these still need to be manually freed in good time in the presence of a garbage collector.
Given your posting history, I suspect you are fully aware of this. However, some developers who don’t know much better who read this, or similar phrases in books extolling the marvels of GC, take phrases like "Full system-wide GC would help because people wouldn’t have to worry about freeing *anything*" and assume *anything* means *anything*, and not just *any memory*.
And I’ve found that some such developers, having been told they don’t need to think about object/resource lifetime and ownership, and never having had much practice at it, find it really hard to do so when they *do* need it.
Now, I’ll agree that C is a bit of a mess in this regard, and we probably shouldn’t have to deal with things at quite that level. But there are intermediates beween C’s malloc()/free() and system-wide GC. RAII for example.
Ah, true. Allow me to modify my statement:
> Full system-wide GC would help because people wouldn’t have to worry about freeing *any* memory, including the various types of strings.
I (obviously) wasn’t thinking about anything except memory, but you raise a good point.
OK, got a second response from the “contact us” address:
> We will not be able to make the immediate
> changes that you have reported because
> the content is acquired by MSDN.
> I have forwarded the issue to the content
> owner of the Web site for review and
> resolution on their end. Please visit the
> site from time to time to check for
> any development.
So I’m not sure what “acquired by MSDN” means, but I suspect it means that “MSDN owns that page, we can’t change it”, based on the second paragraph.
Hopefully somebody that they forwarded this to will be able to at least put a “no, you can’t free this memory, and yes, it’ll leak, but at the moment there’s nothing we can do about that” note on that page. Or release an update that uses a standard allocator, and put a note there that “if the user’s IE version is above X, pass the returned pointer to CoTaskMemFree [or whatever]. Otherwise, you will have to let it leak.”
And not meaning to pick on poor FindMimeFromData, but the return values aren’t documented correctly either. If you pass in a NULL for the url, and a blank (zero length) string for the data, the result is an E_FAIL — but the documentation clearly says that the only return values are NOERROR, E_INVALIDARG, and E_OUTOFMEMORY.
Ah, I get it. I thought the person writing that was not in the MSDN group, and that threw me originally. (I figured they had handed off management of those pages to MSDN or something.) Your version makes a *lot* more sense. :-)
Thanks for the explanation!
When sending messages to window controls (ListBox/LB_ADDSTRING), is the caller/application responsible for removing the rows later, or should a programmer rely on deallocation when destroying the control and/or container dialog?