The subtleties of Create­Stream­On­HGlobal, part 2: Suppressing the deletion of an unknown HGLOBAL

Raymond

Last time, we looked at the easy case of the Create­Stream­On­HGlobal function, where you just hand an HGLOBAL to the stream and wipe your hands of it.

The weird case is where you pass fDelete­On­Release = FALSE. For now, let’s look at the case where you pass a null HGLOBAL, but also pass fDelete­On­Release = FALSE, indicating that you want the stream to create its own HGLOBAL, but not to destroy it on destruction.

The typical use case for this pattern is where you create a stream, write to it, and then extract all the data in the form of an HGLOBAL. To get the HGLOBAL back out, you must call Get­HGlobal­From­Stream. If you don’t do that, then the HGLOBAL inside the stream is leaked, and you have no way to rescue it.

// First, the version without RAII types.
HGLOBAL CreateHGlobalFromStuff()
{
    HGLOBAL hglob = nullptr;
    IStream* stream = nullptr;

    if (SUCCEEDED(
        CreateStreamOnHGlobal(nullptr, FALSE, &stream))) {
        if (FAILED(GetHGlobalFromStream(stream, &hglob))) {
            __fastfail(FAST_FAIL_FATAL_APP_EXIT);
        }
        WriteStuffToStream(stream);
        stream->Release();
    }
    return hglob;
}

void Sample()
{
    HGLOBAL hglob = CreateHGlobalFromStuff();
    if (hglob) {
        DoStuffWith(hglob); // maybe put it on the clipboard
        GlobalFree(hglob);
    }
}

And with RAII:

wil::unique_hglobal CreateHGlobalFromStuff()
{
    wil::unique_hglobal hglob;
    wil::com_ptr<IStream> stream;

    THROW_IF_FAILED(
        CreateStreamOnHGlobal(nullptr, FALSE, &stream));

    FAIL_FAST_IF_FAILED(
        GetHGlobalFromStream(stream.get(), &hglob));

    WriteStuffToStream(stream.get());

    return hglob;
}

void Sample()
{
    wil::unique_hglobal = CreateHGlobalFromStuff();
    DoStuffWith(hglob.get()); // maybe put it on the clipboard
}

The idea here is that you create an HGLOBAL-based stream with no initial memory block, which means that the stream object creates its own empty one. From that, you immediately get the inner HGLOBAL so you can access the data that is being managed. You do this immediately so that the memory block is held in an RAII type so it won’t be leaked if something goes wrong later.

You then write stuff to the stream, which causes it to go into the HGLOBAL. You then throw the stream away and keep the HGLOBAL. The stream’s destruction does not destroy the HGLOBAL because you passed fDelete­On­Release = FALSE.

Note that there is a bit of a scary place: If Get­HGlobal­From­Stream fails, we are kind of stuck. The HGLOBAL inside the stream is going to be leaked, and there’s nothing we can do about it. Fortunately, Get­HGlobal­From­Stream always succeeds if the stream it was given came from the Create­Stream­On­HGlobal function.

The caller takes the resulting HGLOBAL, does something with it, and then frees the HGLOBAL.

It is crucially important that you keep your eye on the stream. You have to be sure that nobody has extended the lifetime of the stream (by calling Add­Ref), because you just freed the HGLOBAL, and any attempt to use the stream beyond that point will be a use-after-free bug.

One mistake people make with passing fDelete­On­Release = FALSE is forgetting that they are now responsible for freeing the HGLOBAL that is inside the stream. If you passed an HGLOBAL of NULL when creating the stream, then you must call Get­HGlobal­From­Stream to find out what HGLOBAL ended up being allocated for the stream, because it’s your responsibility to free it.

// Do not use! This code leaks the global handle.

IStream* stream;
if (SUCCEEDED(CreateStreamOnHGlobal(nullptr,
        FALSE, &stream)) {
    WriteStuffToStream(stream);
    ReadStuffFromStream(stream);
    stream->Release();
}

This code is under the the mistaken belief that the fDelete­On­Release parameter controls not whether the stream frees its internal HGLOBAL but rather whether it frees the HGLOBAL that was passed in. And since they passed nullptr as the incoming HGLOBAL, they certainly don’t want the stream to try to free a null pointer. But that’s not what fDelete­On­Release means.

In the case where you aren’t interested in extending the lifetime of the inner HGLOBAL beyond the lifetime of the stream, just pass fDelete­On­Release = TRUE and let the HGLOBAL be destroyed as part of the natural destruction of the stream.

Next time, we’ll look at what happens if you provide an initial HGLOBAL and combine it with fDelete­On­Release = FALSE.

5 comments

Comments are closed. Login to edit/delete your existing comments

  • Oliver Dewdney

    Could the hGlobal change between calling GetHGlobalFromStream and WriteStuffToStream if it needs to call GlobalReAlloc in the example code?

    • skSdnW

      MSDN is not 100% clear but I assume it always creates a movable block where the handle does not change.

      When you pass in a handle it says: “The handle must be allocated as moveable and nondiscardable” which seems very definitive except later on it says “CreateStreamOnHGlobal will accept a memory handle allocated with GMEM_FIXED, but this usage is not recommended. HGLOBALs allocated with GMEM_FIXED are not really handles and their value can change when they are reallocated.”. We will probably hear more about this in the next post.

      When you pass in NULL it does not specify how the HGLOBAL is created. Unfortunately it also says “If the memory block must be accessed, the memory access calls should be surrounded by calls to GlobalLock and GlobalUnLock.”. It would be nice if that should was changed to a must.

      • Neil Rashbrook

        How else would you get access to the underlying memory block? (Although maybe this dates back to 16-bit protected mode Windows where global allocations were simply their own selectors so being movable makes no practical difference.)

        • Tim Weis

          When passing the GMEM_FIXED flag into GlobalAlloc() then according to the documented contract: “The return value is a pointer.”. In this situation a client can access the underlying memory without a call to GlobalLock(), by reinterpreting the returned HGLOBAL as a pointer. Now obviously, with the internals of CreateStreamOnHGlobal() unspecified, the only safe option is to always GlobalLock() the HGLOBAL, even when there are cases where this is not strictly required.

          I’m guessing that the documentation is less strict in its wording to account for the scenario where non-movable memory is requested. I will also acknowledge that this particular choice of wording can be confusing. Maybe someone in the know can prepare a PR to have the documentation updated.

  • Ismo Salonen

    Shouldn’t Sample() function the call DoStuffWith() have hglobal.get() as argument and not hglob.get() ( in the RAII example) ? May this is nitpicking but imho code examples should be correct, at least otherwise I start wondering what I’m missing and why.

    Otherwise excellent topic, these small details are too often overlooked and then I’ve have to hunt leaked handles .