Why do I get a _BLOCK_TYPE_IS_VALID debug assertion failure when I try to delete a WIC pixel buffer?

A customer had a problem deleting a WIC pixel buffer. Fortunately, they were able to reduce it to a few lines of code.

// Code in italics is wrong.
UINT sourceBufferSize = 0;
BYTE* sourceBuffer = nullptr;

// The next line succeeds.
bitmapLock->GetDataPointer(&sourceBufferSize, &sourceBuffer));

// Omitted code that performs some tasks on the pixels

if (sourceBuffer != nullptr)
    delete sourceBuffer;

"The problem is that when we try to delete the source­Buffer, we get an error from the Visual C++ Debug Library:

Debug Assertion Failed!

Program: Contoso.exe
File: dbgdel.cpp
Line: 52

Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUser)

For more information on how your program can cause an assertion failure, see the Visual C++ documentation on asserts.

"We're not sure what the problem is. Does this mean that we don't need to delete the pixel buffer, and it will be automatically deleted when we release the IWICBitmapLock? I tried leaving the buffer alone, and it doesn't seem to result in a memory leak, but we want to be sure that's the right thing."

The answer is obvious if you are an operating system person: The operating system doesn't know anything about your language runtime's memory allocation strategy. It doesn't try to guess what version of the C++ runtime you're using, and even if it tried to guess, what would it do when faced with a C++ runtime from the future? As far as the operating system is concerned, the delete operator is just some function that is private to your program. It could be called flubber for all the operating system knows. How can the operating system know how to allocate memory so it can be flubbered?

Anyway, the deal is that the pointer you get back from the Get­Data­Pointer method is a pointer to memory that is not owned by you. The lock gives you access to the memory, but all you can do is access the memory. You cannot free it because it was never yours.

The same logic applies to GDI bitmaps. When you create a GDI bitmap, you get an HBITMAP, which represents the bitmap. You can ask GDI to tell you where it put the bitmap pixels by calling Get­Object: a pointer to the bits will be returned in the bmBits member, and you can use that pointer to read or write the pixels of the bitmap. But you can't free the memory. The memory belongs to the HBITMAP.

The IWIC­Bitmap works in a similar way. When you create a WIC bitmap, it allocates some memory to hold the pixels, and that memory belongs to the bitmap. You can call IWIC­Bitmap::Lock, to get an object represented by the IWIC­Bitmap­Lock interface. From the lock object, you can ask for a pointer to the memory that holds the pixels, at which point you can read or write the pixels. That pointer is valid only for the duration of the lock. After you release the lock, the WIC bitmap is permitted to move the memory to somewhere else. (For example, this might happen as part of atlas compaction.)

This is all spelled out and even demonstrated in the sample code that accompanies the IWIC­Bitmap­Lock::Get­Data­Pointer method.

Comments (26)
  1. Antonio Rodríguez says:

    Well, i don’t know much about WIC, but generally, when you make a call that allocates some resource, or gets a pointer or handle to some resource, you must use a matching call to free the resource or handle. There are some exceptions, of course; but this is the general rule.

    I use to indent code between those two calls whenever I can, so I clearly define where the pointer/handle is valid. It’s a little weird, but easy to understand and easier to follow.

    1. ZLB says:

      This should be fairly obvious. Ignoring the concept of buffer ownership, you can’t free something you didn’t allocate unless explicitly told otherwise.

    2. smf says:

      I don’t think you can generalise a rule that any pointer you get from an API must be freed. The Lock/Get/Release pattern is very common. Even more so when calling C++ classes. You wouldn’t do:

      string x = y.Name();
      delete x;

      1. Ben Voigt (Visual Studio and Development Technologies MVP with C++ focus) says:

        std::string is not a pointer, so even the most general form of the rule would not apply. It contains a pointer to content, and it WILL free that pointer… which is one of a great many issues that arise trying to mix std::string instances from different toolchain versions.

        1. smf says:

          You write code like this?

          string x = y.Name();
          char *z = x.c_str();
          delete z;

          1. Brian says:

            A more common case (where you mix string classes with non-class-based code) is where the caller works with a string class, somehow gets a pointer to the string’s buffer and passes it to some non-class aware function (typically way down the call stack) whose behavior includes assuming ownership of the buffer. Yes, the code is just about as bad as your example, but, it’s *much* harder to see.
            A similar problem arises with BSTRs. BSTR behavior is so close to wchar_t* behavior that some folks mix them up. Some mixing and matching of BSTRs and wchar_t* works quite well. Other mixing and matching leads to oddball bugs.

          2. Azarien says:

            It’s error prone to assign the result of c_str() to a variable. c_str() should only be used inside a call, e.g. strlen(s.c_str());

      2. Antonio Rodríguez says:

        What I was talking about is that the reserve and release operations are symmetrical, that you free or release what you have been given (and not anything else). If you receive a handle to an object (a file, a DC…), you don’t free the object’s memory, but you release the handle. In the case exposed in the article, you aren’t allocating memory, you are just given a pointer to it. Following the same pattern, you release the pointer, but you don’t free the block.

  2. Wear says:

    “Code in italics is wrong” but all the code is in italics. Does that mean all the code is wrong?

      1. “Code rendered in the style sheet’s interpretation of the <i> tag is wrong.” is much more cumbersome to write.

        1. smf says:

          I think there is something beautiful about your joke of code in italics is wrong and you post all code in italics, when (on MS browsers at least) the code is not really in italics.

          How about this for less cumbersome?

          // Code is wrong.

          1. There are other posts where there is a mix of italicized and non-italicized code.

      2. Antonio Rodríguez says:

        Let’s bring back Nitpicker’s Corner! Some comments just deserve it.

    1. Yup. It is a general rule on this site that code in italics is wrong. it so happens that all of this code is in italics. (Other times, only some of the code is in italics.)

    2. Smithers says:

      Yes; code is often right or wrong as a whole, especially when memory allocation/deallocation is concerned.

      char * p = new char[100];
      // Do something with p

      Is the first line wrong because it should be using malloc or is the last line wrong because it should be using delete[]? The only thing that can be said for certain is that the block as a whole is wrong.

      And to smf: it looks italic to me. Your machine may be substituting an oblique font in lieu of italics, but comparing to other usages of fixed-width text in the article, the letter shapes are definitely different. Compare the a and f to later mentions of sourceBuffer and IWICBitmap.

      1. smf says:

        When viewed in EDGE and IE it’s oblique type, when viewed in Google Chrome it’s italic type. I have always assumed the Raymond targets his site at MS browsers.

        I don’t know what browser you are using, so I don’t know how your “f” looks.


        1. Smithers says:

          I’m using Chrome on Windows 7. Here ‘f’ gains a tail and ‘a’ changes to the single-storey form.
          Both IE and Firefox don’t change that much, although the font does gain noticeably more serifs, especially at the bottom of the ‘f’.
          Lynx simply displays it in blue, in case anyone was interested.

        2. I overrode Firefox’s defaults to use Calibri, Cambria, and Consolas as the default fonts for web pages, so the text actually looks italicized instead of just being oblique (I’m not sure what it does by default; I did this ages ago). Looking at the stylesheet, the pre element is overriding the font-family to be monospace, instead of using the default fonts listed in bootstrap.min.css (which calls out Consolas explicitly before falling back on Courier New).

          Chrome appears to explicitly use Consolas instead of Courier New, which is curious since it uses Times New Roman and Arial for its other default fonts.

          IE11 and Edge stick to the standard fonts, unless you override them in Internet Options.

    3. Antonio Rodríguez says:

      In case the previous replies aren’t clear enough, the entire block is in italics because the entire block is wrong. As we discuss above, it mixes different allocation/release techniques.

    4. DWalker07 says:

      Yes, all of that code is wrong when taken as a whole.

  3. Ben Hutchings says:

    Even if this function did take ownership of the buffer and the buffer was somehow allocated on the application’s C++ heap, it should then use delete[] to delete an array, not scalar delete. And it should use a smart pointer to avoid leaking.

    1. Ar says:

      Well, the key issue is that we don’t know how the buffer was allocated. Perhaps it was with new, perhaps it was something else like CoTaskAlloc. So don’t use delete or delete[], but read the documentation and use what that says to free the buffer.

      Or not free at all … In this case, the documentation is a bit unclear unfortunately, but it seems indeed the buffer pointer is a const pointer you are not supposed to change or delete. In the end indeed the customer already suspected this, but IMHO I believe MSDN and the API definition itself could have been more clear and hence are the real issue here.

  4. mikeb says:

    For me the general rule is that when API documentation doesn’t say anything about says how you should free a pointer or handle, then you’re not the owner and you shouldn’t try to free the handle or pointer.

    The documentation for `IWICBitmapLock::GetDataPointer` gives an additional clue in that it mentions the valid lifetime of the pointer (without telling you that you’re responsible for freeing/releasing/deleting the resource).

    All that said, I’m not opposed to documentation explicitly saying something along the lines of “the caller is not responsible for freeing this pointer”.

  5. immibis says:

    This is what happens when people learn “I need to free pointers” instead of “I need to free things I allocated with malloc”.

  6. AberAber says:

    I am used to DirectX, which has Lock and Unlock, instead of GetDataPointer. It’s a little more obvious at least that you don’t own the data.

Comments are closed.

Skip to main content