1

I have an MFC based point of sale application that will be presenting a series of images on a customer facing display as the operator is ringing up items.

To handle the presenting of images, I create a window that contains a CStatic control which is displayed on the customer facing monitor. I am using the CImageclass to load the image from a disk file and then setting the image into the CStatic control.

The prototype with hard coded image names looks like:

CImage CImg;
HRESULT  hr;

CONST TCHAR *px;
// determine the image to display based on the current OEP group number being processed.
switch (m_aszCurrentOEPGroup) { 
    case 16:
        hr = CImg.Load( px = PifGetFilePath (_T("PlatedSandwiches.png"), "b", NULL) );
        break;

    case 20:
        hr = CImg.Load( px = PifGetFilePath (_T("modem_after20.png"), "b", NULL) );
        break;

    default:
        hr = CImg.Load( px = PifGetFilePath (_T("modem_after00.png"), "b", NULL) );
        break;
}

if( m_OEPCustDisplayImage.GetBitmap( ) == NULL )
    m_OEPCustDisplayImage.SetBitmap( HBITMAP(CImg) );

The above code is in a class derived from CWnd and contains the member variable m_OEPCustDisplayImage which is defined as CStatic m_OEPCustDisplayImage;. And looking at this, I really do need to check the result variable hr as to whether the Load() worked. And it also looks like Load() can throw an exception so I should consider that as well.

The above source code is in a class member function that is called to pop up the customer facing image display window, load the required image from a file in a specific folder, and then set the image into the CStatic to display the image.

By pop up the display window, what the code actually does is to do a ShowWindow(SW_SHOW) when displaying the image and a ShowWindow(SW_HIDE) when popping down or removing the displayed image.

This source code is displaying the images as expected.

However I am concerned about resource lifetimes especially of the images that have been loaded. I do not want to create a memory leak.

Since the customer facing display window is created once and then exists until the application exits, the destructor for the window is not a mechanism for resource life cycle management.

The CImage is used to load the image from the file and since it is a local object, when the function returns, it's destructor will be called for cleanup.

I am trying to understand the actual lifecycle of an image and what is happening with the resources being used during the following steps:

  • the CImage is defined
  • the image is loaded from a file into the CImage using load()
  • the image is put into the CStatic using the SetBitmap()
  • the CImage object goes out of scope and its destructor called
  • the image in the CStatic is replaced by a new image or with NULL

Some of the lifecycle questions that come to mind are:

  • when the SetBitmap() is used to put the image into the CStatic is a new copy of the image made?
  • when the CImage destructor triggers when the object goes out of scope, is the image resources cleaned up? (I would expect so)
  • what do I need to do with the image in the CStatic when replacing it with a new image?

Currently in the customer facing window pop down code I am just setting the image in the CStatic to NULL with m_OEPCustDisplayImage.SetBitmap(NULL); however it would seem to me that this would be a resource leak. What happens to the image that had been displayed in the CStatic?

The behavior I am seeing of the application is that the requested image is being loaded into and displayed with the CStatic control which is resized to be the same size as the CWnd containing it using ::SetWindowPos(). The window displays and undisplays as expected. The images change as expected.

Since the image is displayed in the CStatic until the next operator action causes the displayed window to undisplay, it would seem that the SetBitmap() method of the CStatic made its own copy of the image.

Do I need to do something like the following as a part of the customer facing window undisplay and pop down?

CGdiObject myTempObj(m_OEPCustDisplayImage.SetBitmap(NULL));

and then let this object go out of scope so as to cleanup any resources?

I have looked at the Output window of Visual Studio after exiting the application and do not see any indication that these images are causing a resource leak from complaints of the debug Visual Studio C++ Runtime however I am not sure that a resource leak of this type, should it exist, would be identified by the C++ Runtime.

Addendum A: Source Changes to Correct Resource Leaks, retest

While looking for whether the images were not being released properly, I was unable to determine this due to another source of resource leaks the result of which I could see in Windows Task Manager that the GDI Object Count was going up however I was unsure as to what was due to the images and what was due to the other resource leak, button pointers not being properly disposed of.

I could see in the Output window of Visual Studio as the application terminated the list of button pointer leaks as well as their locations. However I did not see any indication of the image resources leaking.

I tracked down the reason for the button pointers resource leak and corrected that.

I also modified the source code for the images as follows.

In the code that handles the undisplay or pop down of the window with the CStatic displaying the image I have the following cleanup source code which moves ownership of the image from the CStatic to a CGdiObject which then goes out of scope deleting the image as it does so:

CGdiObject myObj;
myObj.Attach(m_OEPCustDisplayImage.SetBitmap(NULL));

Then in the code that loads first the CImage object with an image from a file and then loads the image into the CStatic object I made the following change using the Detach() method of CImage with SetBitmap() of CStatic to transfer ownership of the image to the CStatic:

if( ! FAILED(hr)) {
    if (m_OEPCustDisplayImage.GetBitmap( ) == NULL )
        m_OEPCustDisplayImage.SetBitmap( CImg.Detach() );
    else {
        CGdiObject myObj;
        myObj.Attach(m_OEPCustDisplayImage.SetBitmap(CImg.Detach()));
    }
}

After doing these two efforts, I was then able to monitor the GDI Object count using the Windows Task Manager while exercising the application in Debug mode. I could see the GDI Object count increase as the windows with images were displayed and then after they undisplayed and popped down, the count went back to the count before the functionality was triggered.

Setting a breakpoint so that the code to transfer the ownership of the image to the CGdiObject with the SetBitmap(NULL) was replaced with just the call to SetBitmap(NULL) resulted in the GDI Object count increasing as expected.

I then recompiled the corrected source as a Release build and exercised the application in this area of functionality while watching the Windows Task Manager display of the GDI Object count. The count behavior was to increase as the windows with the images were displayed and to decrease back to a baseline value when the images were undisplayed.

Richard Chambers
  • 16,643
  • 4
  • 81
  • 106
  • 1
    Are you actually seeing any resource leaks reported? – Andrew Truckle Jun 28 '18 at 16:24
  • It's not clear if `CImg` is on stack or a member. You should call `CImage::Detach` and manually `DeleteObject` the bitmap that's returned from `SetBitmap`. See also [***Problems setting bitmaps to Static Control***](https://stackoverflow.com/questions/37654257/problems-setting-bitmaps-to-static-control-mfc/37657413#37657413) - `STM_SETIMAGE` returns the previously selected `HBITMAP`, or a different one if png file has transparency. – Barmak Shemirani Jun 28 '18 at 16:37
  • @BarmakShemirani `CImg` is on the stack and is a local variable that goes out of scope when the method returns. Thanks for the link as the answer does explain one thing, that the bitmap object is shared between the `CImage` object and the `CStatic` object once I do the `SetBitmap()`. And it also indicates that I now own the object of the returned handle from `SetBitmap()`. The strange thing is that the `CStatic` is displaying the image despite the `CImage` going out of scope. – Richard Chambers Jun 28 '18 at 17:37
  • @AndrewTruckle as I mentioned in the post, I am not seeing any resource leaks pertaining to the image functionality. I do have a resource leak that is linked to another place in the code that I can also see with the Task Manager application. And unfortunately that resource leak, windows for buttons, is exercised at the same time the image load and display is exercised so watching handles and GDI objects increase doesn't help much. – Richard Chambers Jun 28 '18 at 17:40

1 Answers1

1

Above code is using a png file which can have transparency. This is a special case where SetBitmap ends up creating a different bitmap handle. The image will remain even after the original bitmap handle is destroyed. This method will otherwise fail, for example for 24-bit bitmap.

SetBitmap uses STM_SETIMAGE api described with the following remarks:

With Windows XP, if the bitmap passed in the STM_SETIMAGE message contains pixels with nonzero alpha, the static control takes a copy of the bitmap. This copied bitmap is returned by the next STM_SETIMAGE message. The client code may independently track the bitmaps passed to the static control, but if it does not check and release the bitmaps returned from STM_SETIMAGE messages, the bitmaps are leaked.

You are calling SetBitmap only once, based on testing for GetBitmap. You are leaking 1 resource handle (allowed limit is 10,000) This might be difficult to detect, but it's there. It can run in to problem if you open and close the dialog many times, within the same process.

It is preferable to use CImage::Detach instead of HBITMAP(cimage_object)

The example shown below will not have resource leaks and works with other images. It uses an additional variable bitmap_cleanup to clean the bitmap handle at the end.

class CMyDialog : public CDialogEx
{
    CStatic m_OEPCustDisplayImage;
    CBitmap bitmap_cleanup;
    ...
};

BOOL CMyDialog::OnInitDialog()
{
    CDialogEx::OnInitDialog();

    CImage img;
    HRESULT hr = img.Load(L"test.png");
    if(SUCCEEDED(hr))
    {
        HBITMAP hbitmap = img.Detach();
        //CImage is no longer responsible for cleanup

        HBITMAP holdbmp = m_OEPCustDisplayImage.SetBitmap(hbitmap);

        //we have to cleanup previous handle if any
        if(holdbmp)
            DeleteObject(holdbmp);

        //pass the handle to bitmap_cleanup, to cleanup on exit
        bitmap_cleanup.Attach(hbitmap);
    }

    return TRUE;
}
Barmak Shemirani
  • 30,904
  • 6
  • 40
  • 77
  • It looks like the note you have in block quotes is from the `STM_SETIMAGE` topic rather than the `STM_GETIMAGE` topic. The wording is a bit confusing about Windows XP but XP is not something I worry about anymore. What I take away from this is the line **The client is responsible to delete any bitmap sent to a static control.** Also see Addendum A that I added to my posting. Thank you for your time as well as the original linked to stackoverflow posting which helped a lot. – Richard Chambers Jun 28 '18 at 19:54
  • Right, I mixed that up. I fix that, I also changed the test to `if(SUCCEEDED(hr)){...}` that's more reliable. – Barmak Shemirani Jun 28 '18 at 20:02