November, 2009

  • The Old New Thing

    The difference between assignment and attachment with ATL smart pointers

    • 15 Comments

    Last time, I presented a puzzle regarding a memory leak. Here's the relevant code fragment:

    CComPtr<IStream> pMemoryStream;
    CComPtr<IXmlReader> pReader;
    UINT nDepth = 0;
    
    //Open read-only input stream
    pMemoryStream = ::SHCreateMemStream(utf8Xml, cbUtf8Xml);
    

    The problem here is assigning the return value of SHCreateMemStream to a smart pointer instead of attaching it.

    The SHCreateMemStream function creates a memory stream and returns a pointer to it. That pointer has a reference count of one, in accordance with COM rules that a function which produces a reference calls AddRef, and the responsibility is placed upon the recipient to call Release. The assignment operator for CComPtr<T> is a copy operation: It AddRefs the pointer and saves it. You're still on the hook for the reference count of the original pointer.

    ATLINLINE ATLAPI_(IUnknown*) AtlComPtrAssign(IUnknown** pp, IUnknown* lp)
    {
            if (lp != NULL)
                    lp->AddRef();
            if (*pp)
                    (*pp)->Release();
            *pp = lp;
            return lp;
    }
    
    template <class T>
    class CComPtr
    {
    public:
            ...
    
            T* operator=(T* lp)
            {
                    return (T*)AtlComPtrAssign((IUnknown**)&p, lp);
            }
    

    Observe that assigning a T* to a CComPtr<T> AddRefs the incoming pointer and Releases the old pointer (if any). When the CComPtr<T> is destructed, it will release the pointer, undoing the AddRef that was performed by the assignment operator. In other words, assignment followed by destruction has a net effect of zero on the pointer you assigned. The operation behaves like a copy.

    Another way of putting a pointer into a CComPtr<T> is with the Attach operator. This is a transfer operation:

            void Attach(T* p2)
            {
                    if (p)
                            p->Release();
                    p = p2;
            }
    

    Observe that there is no AddRef here. When the CComPtr<T> is destructed, it will perform the Release, which doesn't undo any operation performed by the Attach. Instead, it releases the reference count held by the original pointer you attached.

    Let's put this in a table, since people seem to like tables:

    Operation Behavior Semantics
    Attach() Takes ownership Transfer semantics
    operator=() Creates a new reference Copy semantics

    You use the Attach method when you want to assume responsibility for releasing the pointer (ownership transfer). You use the assignment operator when you want the original pointer to continue to be responsible for its own release (no ownership transfer).

    There is also a Detach method which is the opposite of Attach: Detaching a pointer from the CComPtr<T> means "I am taking over responsibility for releasing this pointer." The CComPtr<T> gives you its pointer and then forgets about it; you're now on your own.

    The memory leak in the code fragment above occurs because the assignment operator has copy semantics, but we wanted transfer semantics, since we want the smart pointer to take the responsibility for releasing the pointer when it is destructed.

    pMemoryStream.Attach(::SHCreateMemStream(utf8Xml, cbUtf8Xml));
    

    The CComPtr<T>::operator=(T*) method is definitely one of the more dangerous methods in the CComPtr<T> repertoire, because it's so easy to assign a pointer to a smart pointer without giving it a moment's thought. (Another dangerous method is the T** CComPtr<T>::operator&(), but at least that has an assertion to try to catch the bad usages. Even nastier is the secret QI'ing assignment operator.) I have to say that there is merit to Ben Hutchings' recommendation simply not to allow a simple pointer to be assigned to a smart pointer, precisely because the semantics are easily misunderstood. (The boost library, for example, follows Ben's recommendation.)

    Here's another exercise based on what you've learned:

    Application Verifier told us that we have a memory leak, and we traced it back to the function GetTextAsInteger.

    BSTR GetInnerText(IXMLDOMNode *node)
    {
        BSTR bstrText = NULL;
        node->get_text(&bstrText);
        return bstrText;
    }
    
    DWORD GetTextAsInteger(IXMLDOMNode *node)
    {
        DWORD value = 0;
    
        CComVariant innerText = GetInnerText(node);
        hr = VariantChangeType(&innerText, &innerText, 0, VT_UI4);
        if (SUCCEEDED(hr))
        {
            value = V_UI4(&innerText);
        }
    
        return value;
    }
    

    Obviously, the problem is that we passed the same input and output pointers to VariantChangeType, causing the output integer to overwrite the input BSTR, resulting in the leak of the BSTR. But when we fixed the function, we still got the leak:

    DWORD GetTextAsInteger(IXMLDOMNode *node)
    {
        DWORD value = 0;
    
        CComVariant innerText = GetInnerText(node);
        CComVariant textAsValue;
        hr = VariantChangeType(&innerText, &textAsValue, 0, VT_UI4);
        if (SUCCEEDED(hr))
        {
            value = V_UI4(&textAsValue);
        }
    
        return value;
    }
    

    Is there a leak in the VariantChangeType function itself?

    Hint: It is in fact explicitly documented that the output parameter to VariantChangeType can be equal to the input parameter, which results in an in-place conversion. There was nothing wrong with the original call to VariantChangeType.

  • The Old New Thing

    We're using a smart pointer, so we can't possibly be the source of the leak

    • 29 Comments

    A customer reported that there was a leak in the shell, and they included the output from Application Verifier as proof. And yup, the memory that was leaked was in fact allocated by the shell:

    VERIFIER STOP 00000900 : pid 0x3A4: A heap allocation was leaked.
    
            497D0FC0 : Address of the leaked allocation.
            002DB580 : Adress to the allocation stack trace.
            0D65CFE8 : Address of the owner dll name.
            6F560000 : Base of the owner dll.
    
    1: kd> du 0D65CFE8
    0d65cfe8  "SHLWAPI.dll"
    
    1: kd> !heap -p -a 497D0FC0
    ...
        ntdll!RtlpAllocateHeap+0x0003f236
        ntdll!RtlAllocateHeap+0x0000014f
        Kernel32!LocalAlloc+0x0000007c
        shlwapi!CreateMemStreamEx+0x00000043
        shlwapi!CreateMemStream+0x00000012
        <Unloaded_xyz.dll>+0x000642de
        <Unloaded_xyz.dll>+0x0005e2af
        <Unloaded_xyz.dll>+0x0002d49a
        <Unloaded_xyz.dll>+0x0002a0fd
        <Unloaded_xyz.dll>+0x000289cb
        <Unloaded_xyz.dll>+0x0002a25c
        <Unloaded_xyz.dll>+0x00027225
        <Unloaded_xyz.dll>+0x0002252b
        <Unloaded_xyz.dll>+0x00025394
        <Unloaded_xyz.dll>+0x0004d70f
        Kernel32!BaseThreadInitThunk+0x0000000d
        ntdll!RtlUserThreadStart+0x0000001d
    
    1: kd> dps 002DB580
    shlwapi!CreateMemStreamEx+0x43
    shlwapi!CreateMemStream+0x12
    <Unloaded_xyz.dll>+0x642de
    <Unloaded_xyz.dll>+0x5e2af
    <Unloaded_xyz.dll>+0x2d49a
    <Unloaded_xyz.dll>+0x2a0fd
    <Unloaded_xyz.dll>+0x289cb
    <Unloaded_xyz.dll>+0x2a25c
    <Unloaded_xyz.dll>+0x27225
    <Unloaded_xyz.dll>+0x2252b
    <Unloaded_xyz.dll>+0x25394
    <Unloaded_xyz.dll>+0x4d70f
    Kernel32!BaseThreadInitThunk+0xd
    ntdll!RtlUserThreadStart+0x1d
    

    On the other hand, SHCreateMemStream is an object creation function, so it's natural that the function allocate some memory. The responsibility for freeing the memory belongs to the caller.

    We suggested that the customer appears to have leaked the interface pointer. Perhaps there's a hole where they called AddRef and managed to avoid the matching Release.

    "Oh no," the customer replied, "that's not possible. We call this function in only one place, and we use a smart pointer, so a leak is impossible." The customer was kind enough to include a code snippet and even highlighted the lines that proved they weren't leaking.

    CComPtr<IStream> pMemoryStream;
    CComPtr<IXmlReader> pReader;
    UINT nDepth = 0;
    
    //Open read-only input stream
    pMemoryStream = ::SHCreateMemStream(utf8Xml, cbUtf8Xml);
    

    The exercise for today is to identify the irony in the highlighted lines.

    Hint. Answers (and more discussion) tomorrow.

  • The Old New Thing

    News flash: Healthy people live longer

    • 15 Comments

    Researchers have determined that people in good physical condition live longer. Who'd'a thunk it?

  • The Old New Thing

    How do I move the Windows.edb and other search index files?

    • 7 Comments

    Nothing profound today, just a little tip.

    My customer is looking out for a way to change the location of the windows.edb file to another (larger) drive.

    From the Indexing Options Control Panel, click Advanced, and then under Index location, click Select new.

  • The Old New Thing

    We found the author of Notepad, sorry you didn't go to the award ceremony

    • 34 Comments

    I've received independent confirmations as to the authorship of Notepad, so I'm inclined to believe it. Sorry you didn't get to go to the award ceremony.

    The original author of Notepad also served as the development manager for Windows 95. His job was to herd the cats that made up the programmers who worked on Windows 95, a job which you can imagine falls into the "not easy" category.

    After Windows 95, he retired from the software industry and became a high school science teacher. At a social event some years later, I met him again and asked about the transition from software development manager to high school science teacher.

    His response: "You'd be surprised how many of the skills transfer."

  • The Old New Thing

    How to tell when your patent has been approved

    • 10 Comments

    There are a variety of ways of finding out when your patent is granted, but the quickest mechanism is to check your mailbox. But the thing to look for is not what you might think.

    Even before you receive word from your company's patent department, you will start receiving junk mail delivered to your home address from companies that sell patent-related novelties, pointless trinkets like pencils and mugs with your patent number on it. Nevermind that you can probably order personalized pencils for much less than what the patent-chasers were offering.

  • The Old New Thing

    How to pretend that you attended my talk at UIUC Reflections|Projections 2009

    • 12 Comments

    Step 1: Buy a 1.55-ounce Hershey's Milk Chocolate Bar from a convenience store, supermarket, or (if truly desperate) online.

    Step 2: Print out this candy bar wrapper.

    Step 3: Trim wrapper on registration marks and wrap around candy bar.

    Step 4: Stay up late the night before you plan on watching the video by partying with Ryan North and teaching him how to play beer pong.

    Step 5: Force yourself to wake up the next morning and watch the recorded video of my talk while trying desperately to stay awake. The candy bar might help.

    Note: Although most steps are optional, they are essential if you want an accurate simulation.

  • The Old New Thing

    Why does shlwapi import a nonexistent function?

    • 27 Comments
    Commenter charless asks why shlwapi.dll imports a nonexistent function from mpr.dll, which shows up in dependency tools as a broken import.

    Because that function did exist at one point, although it doesn't exist any more.

    The function in question was available only on Windows 95-series versions of Windows. It never existed on Windows NT or any of its successors. But remember that shlwapi.dll was originally developed for Internet Explorer, which ran on Windows 95 as well as Windows NT. Internet Explorer checked the operating system and called the Windows 95-only function only after verifying that it was running on Windows 95. If it was running on Windows NT, then it never called the function and therefore never stepped on the land mine known as ERROR_PROC_NOT_FOUND.

    Okay, so why does shlwapi still link to the function long after the Windows 95 series of operating systems have become obsolete?

    Removing a function, even a function that doesn't do anything, even an undocumented function that doesn't do anything, is a dangerous endeavor. Suppose you have a program that links to the function, but just like Internet Explorer, it is clever and checks whether it is running on Windows NT before calling it. If you remove the useless function from shlwapi, then that program will fail to load, even though it never calls the offending function, and now you have an application compatibility problem on your hands.

    Since it's a small function that doesn't do anything, it's a lot less risky simply to leave the function in.

    Even though it doesn't do anything except fail.

  • The Old New Thing

    What a drag: You can be a drag in managed code, too

    • 12 Comments

    David Anson digests my earlier series on virtual drag/drop and translates it into managed code. His example of dragging his entire RSS feed is an excellent illustration of dragging dynamically-generated virtual content. (I didn't use an example like that because the purpose of the What a drag series was to get something done in the least amount of code, and generating a stream from a URL takes an awful lot of code when doing it from the unmanaged side, which would ultimately detract from the point of the example.)

    Bonus: He takes the example further by adding asynchronous support.

  • The Old New Thing

    You thought reasoning about signals was bad, reasoning about a total breakdown of normal functioning is even worse

    • 17 Comments

    A customer came to the Windows team with a question, the sort of question which on its face seems somewhat strange, which is itself a sign that the question is merely the tip of a much more dangerous iceberg.

    Under what circumstances will the GetEnvironmentVariable function hang?

    This is kind of an open-ended question. I mean, for example, somebody might sneak in and call SuspendThread on your thread while GetEnvironmentVariable is running, which will look like a hang because the call never completes because the thread is frozen.

    But the real question for the customer is, "What sort of problem are you seeing that is manifesting itself in an apparent hang in the GetEnvironmentVariable function?"

    The customer was kind enough to elaborate.

    We have a global unhandled exception filter in our application so we can log all failures. After we finish logging, we call ExitProcess, but we find that the application never actually exits. If we connect a debugger to the stuck application, we see it hung in GetEnvironmentVariable.

    Your gut response should be, "Holy cow, I'm surprised you even got that far!"

    This isn't one of those global unhandled exception filters that got installed because your program plays some really clever game with exceptions, No, this is an "Oh no, my program just crashed and I want to log it" exception handler. In other words, when this exception handler "handles" an exception, it's because your program has encountered some sort of serious internal programming error for which the program did not know how to recover. We saw earlier that you can't do much in a signal handler because you might have interrupted a block of code which was in the middle of updating some data structures, leaving them momentarily inconsistent. But this exception filter is in an even worse state: Not only is there a good chance that the program is in the middle of updating something and left it in an inconsistent state, you are in fact guaranteed that the system is in a corrupted state.

    Why is this a guarantee? Because if the system were in a consistent state, you wouldn't have crashed!

    Programming is about establishing invariants, perturbing them, and then re-establishing them. It is a game of stepping-stone from one island of consistency to another. But the code that does the perturbing and the re-establishing assumes that it's starting from a consistent state to begin with. For example, a function that removes a node from a doubly-linked list manipulates some backward and forward link pointers (temporarily violating the linked list invariant), and then when it's finished, the linked list is back to a consistent state. But this code assumes that the linked list is not corrupted to begin with!

    Let's look again at that call to ExitProcess. That's going to detach all the DLLs, calling each DLL's DllMain with the DLL_PROCESS_DETACH notification. But of course, those DllMain are going to assume that the data structures are intact and nothing is corrupted. On the other hand, you know for a fact that these prerequisites are not met—the program crashed precisely because something is corrupted. One DLL might walk a linked list—but you might have crashed because that linked list is corrupted. Another DLL might try to delete a critical section—but you might have crashed because the data structure containing the critical section is corrupted.

    Heck, the crash might have been inside somebody's DLL_PROCESS_DETACH handler to begin with, for all you know.

    "Yeah, but the documentation for TerminateProcess says that it does not clean up shared memory."

    Well, it depends on what you mean by clean up. The reference count on the shared memory is properly decremented when the handle is automatically closed as part of process cleanup, and the shared memory will be properly freed once there are no more references to it. It is not cleaned up in the sense of "corruption is repaired"—but of course the operating system can't do that because it doesn't know what the semantics of your shared memory block are.

    But this is hardly anything to get concerned about because your program doesn't know how to un-corrupt the data either.

    "It also says that DLLs don't receive their DLL_PROCESS_DETACH notification."

    As we saw before, this is a good thing in the case of a corrupted process, because the code that runs in DLL_PROCESS_DETACH assumes that your process has not been corrupted in the first place. There's no point running it when you know the process is corrupted. You're just making a bad situation worse.

    "It also says that I/O will be in an indeterminate state."

    Well yeah, but that's no worse than what you have now, which is that your I/O is in an indeterminate state. You don't know what buffers your process hasn't flushed, but since your process is corrupted, you have no way of finding out anyway.

    "Are you seriously recommending that I use TerminateProcess to exit the last chance exception handler?!?"

    Your process is unrecoverably corrupted. (This is a fact, because if there were a way to recover from it, you would have done it instead of crashing.) What other options are there?

    Quit while you're behind.

Page 2 of 4 (34 items) 1234