Changes to how we display data

By , June 12, 2020 11:59 am

We’ve changed the display of data on all of our tree controls that are used to display callstacks and call trees.

The old method of display, everything was displayed in black text at normal font weight.

The new method of display, different components of the text are displayed in their own colour, with optional bold, italic, underline or strikethrough, with an optional change in font face. In practice, at present we’re only changing the display colour and turning bold on/off.

We’re using this new display technology to leave some items neutral, emphasise some items and de-emphasise other items. This change has made quite a difference in readability of the display. Right now there is no ability for the user of the software to edit these settings to change the prioritisation and colour scheme we’ve chosen. We’ll provide this capability in a future release.

I’m going to show some before and after images so that you can see the difference for each tool.

C++ Memory Validator

For C++ Memory Validator, we’ve changed both function and filename to be displayed in bold. Type, thread name, timestamp, module name, and symbol name are displayed in different colours. Address is displayed in a de-emphasised colour.

Before


After


Before


After


.Net Memory Validator

For .Net Memory Validator, we’ve changed both function and filename to be displayed in bold. Age, Generation, thread name, timestamp, module name, and symbol name are displayed in different colours. Address is displayed in a de-emphasised colour.

Before


After


Before


After


Thread Validator

For Thread Validator, we’ve changed both function and filename to be displayed in bold. Thread name, timestamp, module name, and symbol name are displayed in different colours.

Before


After


Before


After


Performance Validator

For Performance Validator, we’ve changed both function and filename to be displayed in bold. Module name, and symbol name are displayed in different colours.

Before


After


Coverage Validator

For Coverage Validator, the branches, files and directory displays have had a few tweaks to de-emphasis address information.

Before

After

Bug Validator

For Bug Validator, we’ve re-ordered the display information so that the symbol comes before the filename. Both function and filename are displayed in bold. Timestamp, thread name, module name, and symbol name are displayed in different colours.

Before

After

Memory leak in CPngImage

By , June 1, 2020 9:37 am

A memory leak in a surprising place

We’ve recently been doing some work switching our resources in our programs from BMP (using CBitmap) to PNG (using CPngImage).

At some point we got around to dog-fooding, which we do with our tools all the time, and we were surprised to see memory leaks (in the form of HGLOBAL handles) in our tools being reported by C++ Memory Validator.


We took a look and found they were coming from CPngImage::LoadFromBuffer(). Here’s the code, with the leaking lines highlighted.

BOOL CPngImage::LoadFromBuffer(LPBYTE lpBuffer, UINT uiSize)
{
        ASSERT(lpBuffer != NULL);

        HGLOBAL hRes = ::GlobalAlloc(GMEM_MOVEABLE, uiSize); // this line leaks
        if (hRes == NULL)
        {
                return FALSE;
        }

        IStream* pStream = NULL;
        LPVOID lpResBuffer = ::GlobalLock(hRes);             // this line leaks
        ASSERT (lpResBuffer != NULL);

        memcpy(lpResBuffer, lpBuffer, uiSize);

        HRESULT hResult = ::CreateStreamOnHGlobal(hRes, FALSE, &pStream);

        if (hResult != S_OK)
        {
                return FALSE;
        }

        if (CMFCToolBarImages::m_bMultiThreaded)
        {
                CMFCToolBarImages::m_CriticalSection.Lock();
        }

        if (m_pImage == NULL)
        {
                m_pImage = new CImage;
                ENSURE(m_pImage != NULL);
        }

        m_pImage->Load(pStream);
        pStream->Release();

        BOOL bRes = Attach(m_pImage->Detach());

        if (CMFCToolBarImages::m_bMultiThreaded)
        {
                CMFCToolBarImages::m_CriticalSection.Unlock();
        }

        return bRes;
}

Verifying the memory leak

At first we thought C++ Memory Validator might have made a mistake, as it seemed unlikely that such a well used class would contain a mistake like a memory leak.

To check if this was correct memory leak report or a FALSE positive we created a test program where we can repeatedly create and destroy images in rapid succession. If the leak is real the toy program will soon fail to allocate memory. If the leak is a false positive by C++ Memory Validator, the toy program will run forever with no problems. The toy program demonstrated the leak is real – after just over 65,000 loads of an image with CPngImage, all GlobalAlloc() allocations fail.

The test program allows you to repeatedly load a BMP, a PNG into CPngImage and a PNG into an svlPngImage. The image is destroyed after loading. You can specify how many times to perform the load, 1, 10, 100, 1000, 10000, 64K and 100,000 times.


We have verified that his memory leak is present in all versions of CPngImage that ship with all versions of Visual Studio (up to and including VS2019, we haven’t looked at VS2021 yet).

Fixing the memory leak

The fix is to add two lines just before the return bRes; statement.

        ::GlobalUnlock(hRes);
        ::GlobalFree(hRes);

Unfortunately CPngImage loads it data from methods that are not virtual, so we couldn’t replace them in the implementation. We created a drop in replacement for CPngImage called svlPngImage. It’s identical to the CPngImage class with the exception that the CMFCToolBarImages calls are commented out, and the two additional lines to prevent the memory leak are added. If you’d like to use svlPngImage the source code for this drop in replacement is in the download.

Test Program Source Code

You can download the test program source code and project files.

Update, after response from Microsoft

I was surprised when I got a reply saying this couldn’t happen because the second parameter to ::CreateStreamOnHGlobal() was TRUE. I searched my machine again, and it was FALSE. I then searched a different machine and it was TRUE. So most likely this bug did exist (and we’re using the version of Visual Studio that has it) but has been fixed in a more recent service pack. My claim that the bug was in all versions of Visual Studio was incorrect because I was checking for the absence of the memory freeing calls without checking the value passed to ::CreateStreamOnHGlobal() was FALSE. The drop in replacement class is still valid.

If you’re using CPngImage, check CPngImage::LoadFromBuffer() and if the second parameter passed to ::CreateStreamOnHGlobal() is FALSE, use our svlPngImage class instead. Otherwise carry on as you are.

Update 2, after further response from Microsoft

Microsoft has modified this function to improve the error handling to fix a memory leak if ::CreateStreamOnHGlobal() fails, in response to this bug report, which prompted them to reexamine the function. Ironic in that the report I made, although demonstrating a real bug, that bug had already been fixed, but it still prompted a fresh look at the function behaviour.

Panorama Theme by Themocracy