Memory Leaks found in an MFC / COM / GDI application (HTML thumbnail image capture class)

Hi, I'm new to Experts-Exchange and this would be my first question.  Thanks in advance, please let me know if the format of my question can be improved to make it easier for the experts :)

I am a beginner in using MFC and COM and I am following up on a windows application.  One of the features is to be able to generate an image from a URL.  I have included the code of the function in the thumbnail capture class (CCreateHTMLImage).  (The original source for this code can be found at http://www.codeproject.com/KB/IP/htmlimagecapture.aspx - I have modified the CreateImage function, integrating with IECapt's SaveSnapShot method found here http://iecapt.sourceforge.net/ ).

When the user chooses to generate a thumbnail capture of a webpage, the application will do the following routine inside a CView derived class:
m_pHTMLImage is a pointer set to NULL initially

            if(m_pHTMLImage == NULL) {
                  OutputDebugString("Initializing m_pHTMLImage\n");
                  m_pHTMLImage = new CCreateHTMLImage();
                  m_pHTMLImage->Create(this);
            }

            if(!m_pHTMLImage -> CreateImage(_T("http://")+URL, AppDirectory + _T("\\temp.jpg"), CSize(1024, 768), CSize(360,300)))
                  return;

            It will then call a function LoadPic(AppDirectory + _T("\\temp.jpg"));

LoadPic(CString PicPath)
{
      CFile file;
      if(!file.Open(PicPath, CFile::modeRead|CFile::shareDenyWrite|CFile::typeBinary))
            return FALSE;
      if(m_dwPicLen != 0)
            delete[] m_BPicData; //if pic already exists, clear and load new data
      m_dwPicLen = (DWORD)(file.GetLength());                                                
      if(!m_Pic.Load(file.GetFilePath()))
            return FALSE;
      m_bIsPic = TRUE; //set display mode to pic
      m_BPicData = new BYTE[m_dwPicLen];
      file.Read(m_BPicData, m_dwPicLen);
      file.Close();
      return TRUE;
}

the memory usage jumps from ~10,000K to ~100,000K, should it be using this much memory?
subsequent calls to this routine of generating an image will raise mem usage by ~10,000K
I only create one instance of CCreateHTMLImage and will do the following routine when the application closes in the CView class' destructor

      if(m_pHTMLImage) {  
            m_pHTMLImage->DestroyWindow();  
            delete m_pHTMLImage;  // EL
            m_pHTMLImage = NULL;  
      }

Are there any problems regarding the Allocation and Release of Interfaces or in Getting and Releasing/Deleting DCs in the CreateImage Function? would i have to delete or release hdcMain?

I have used PurifyPlus and I have attached the results as images.
The leaks seem to come from DLLs, does it mean that it is not related to the classes I am working on?

More recent debug results are from boundschecker.

I am also getting many many of the following errors when i generate a thumbnail
First-chance exception at 0x30195840 in czWeb.exe: 0x80000001: Not implemented.
First-chance exception at 0x30195196 in czWeb.exe: 0x80000001: Not implemented.
Any idea what could generate these errors ?

Any help would be greatly appreciated.  Please let me know if anything was unclear or if you need more information

Thanks

BOOL CCreateHTMLImage::CreateImage(LPCTSTR szSrcFilename, LPCTSTR szDestFilename, CSize srcSize, CSize outputSize)
{
	USES_CONVERSION;
	ASSERT(GetSafeHwnd());
	ASSERT(IsWindow(GetSafeHwnd()));
	ASSERT(szSrcFilename);
	ASSERT(AfxIsValidString(szSrcFilename));
	ASSERT(szDestFilename);
	ASSERT(AfxIsValidString(szDestFilename));
 
	CRect rect(CPoint(0, 0), srcSize);
 
	//	The WebBrowswer window size must be set to our srcSize
	//	else it won't render everything
	MoveWindow(&rect);
	m_pBrowserWnd.MoveWindow(&rect);
 
	COleVariant	  vUrl(szSrcFilename, VT_BSTR),
				  vFlags(long(navNoHistory | navNoReadFromCache | navNoWriteToCache), VT_I4),
				  vNull(LPCTSTR(NULL), VT_BSTR);
	COleSafeArray vPostData;
 
	if (m_pBrowser->Navigate2(&vUrl, &vFlags, &vNull, &vPostData, &vNull) == S_OK)
		//	We have to pump messages to ensure the event handler (DocumentComplete)
		//	is called.
		RunModalLoop();
	else
		return FALSE;
 
//	We only get here when DocumentComplete has been called, which calls EndModalLoop
	//	and causes RunModalLoop to exit.
 
 
	IHTMLDocument3* pDocument3 = NULL;
        IHTMLDocument2* pDocument  = NULL;
        IHTMLElement2* pElement2   = NULL;
        IHTMLElement* pElement     = NULL;
        IViewObject2* pViewObject  = NULL;
        IDispatch* pDispatch       = NULL;
        //IDispatch* pWebBrowserDisp = NULL;
 
        HRESULT hr;
        long bodyHeight;
        long bodyWidth;
        long rootHeight;
        long rootWidth;
        long height;
        long width;
 
        hr = m_pBrowser->get_Document(&pDispatch);
		if (hr == S_OK) {  // Need to release pDispatch
 
			hr = pDispatch->QueryInterface(IID_IHTMLDocument2, (void**) &pDocument);
			if (hr == S_OK) {  // Need to release pDocument
 
				hr = pDocument->get_body(&pElement);
				if (hr == S_OK) {  // Need to release pElement
 
					hr = pElement->QueryInterface(IID_IHTMLElement2, (void**) &pElement2);
					if (hr == S_OK) {  // Need to release pElement2
 
						hr = pElement2->get_scrollHeight(&bodyHeight);
						if (FAILED(hr))
							return true;
 
						hr = pElement2->get_scrollWidth(&bodyWidth);
						if (FAILED(hr))
							return true;
 
						pElement2->Release();
					}
					pElement->Release();
				}
				pDocument->Release();
			}
 
			hr = pDispatch->QueryInterface(IID_IHTMLDocument3, (void**) &pDocument3);
			if (hr == S_OK) {  // Need to release pDocument3
 
				hr = pDocument3->get_documentElement(&pElement);
				if (hr == S_OK) {  // Need to release pElement
 
					hr = pElement->QueryInterface(IID_IHTMLElement2, (void**) &pElement2);
					if (hr == S_OK) {  // Need to release pElement2
 
						hr = pElement2->get_scrollHeight(&rootHeight);
						if (FAILED(hr))
							return true;
 
						hr = pElement2->get_scrollWidth(&rootWidth);
						if (FAILED(hr))
							return true;
						pElement2->Release();
					}
					pElement->Release();
				}
				pDocument3->Release();
			}
 
 
			width = bodyWidth;
			height = rootHeight > bodyHeight ? rootHeight : bodyHeight;
 
			MoveWindow(0, 0, width, height, TRUE);      
			::MoveWindow(m_pBrowserWnd.GetSafeHwnd(), 0, 0, width, height, TRUE);
 
			hr = m_pBrowser->QueryInterface(IID_IViewObject2, (void**) &pViewObject);
			if (hr == S_OK) {  // Need to release pViewObject
				BITMAPINFOHEADER bih;
				BITMAPINFO bi;
				RGBQUAD rgbquad;
 
				ZeroMemory(&bih, sizeof(BITMAPINFOHEADER));
				ZeroMemory(&rgbquad, sizeof(RGBQUAD));
 
				bih.biSize          = sizeof(BITMAPINFOHEADER);
				bih.biWidth         = width;
				bih.biHeight        = height;
				bih.biPlanes        = 1;
				bih.biBitCount      = 24;
				bih.biClrUsed       = 0;
				bih.biSizeImage     = 0;
				bih.biCompression   = BI_RGB;
				bih.biXPelsPerMeter = 0;
				bih.biYPelsPerMeter = 0;
 
				bi.bmiHeader = bih;
				bi.bmiColors[0] = rgbquad;
 
				CDC* dc = GetDC();
				HDC hdcMain = *dc;
 
				if (!hdcMain)
					return true;
 
				HDC hdcMem = CreateCompatibleDC(hdcMain);
 
				if (!hdcMem)
					return true;
 
				char* bitmapData = NULL;
				HBITMAP hBitmap = CreateDIBSection(hdcMain, &bi, DIB_RGB_COLORS, (void**)&bitmapData, NULL, 0);
 
				if (!hBitmap) {
					// TODO: cleanup
					return true;
				}
 
				// Save original bitmap, note this is needed to be reselected to preven GDI resource leak // EL
				HGDIOBJ oldHBitmap = GetCurrentObject(hdcMem, OBJ_BITMAP);
				if(oldHBitmap == NULL) {
					OutputDebugString("GetCurrentObj failed\n");
					return true;
				}
 
				SelectObject(hdcMem, hBitmap);
 
				RECTL rcBounds = { 0, 0, width, height };
				hr = pViewObject->Draw(DVASPECT_CONTENT, -1, NULL, NULL, hdcMain, hdcMem, &rcBounds, NULL, NULL, 0);
				if (SUCCEEDED(hr)) {
					CImage image;
					image.Create(outputSize.cx, outputSize.cy, 24);
					CImageDC imageDC(image);
					::StretchBlt(imageDC, 0, 0, outputSize.cx, outputSize.cy, hdcMem, 0, 0, width, height, SRCCOPY);
					//::BitBlt(imageDC, 0, 0, width, height, hdcMem, 0, 0, SRCCOPY);
					image.Save(szDestFilename);
				}
				
				SelectObject(hdcMem, oldHBitmap);
				DeleteObject(hBitmap);
				DeleteDC(hdcMem);
				//DeleteDC(hdcMain);
				ReleaseDC(dc);
				pViewObject->Release();
			}
			pDispatch->Release();
		}
		return true;
 
}

Open in new window

bcsummary.jpg
bc1.jpg
bc2.jpg
bc3.jpg
purify2.jpg
purify3.jpg
purify4.jpg
LVL 1
DrivenXAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

evilrixSenior Software Engineer (Avast)Commented:
The following info may help you to track down if you have any heap related memory leaks.

You can use _CrtSetDbgFlag to enable CRT heap allocation debugging. This should be at the very start of your program.
E.g. _CrtSetDbgFlag ( _CrtSetDbgFlag( _CRTDBG_REPORT_FLAG ) | _CRTDBG_LEAK_CHECK_DF );
http://msdn2.microsoft.com/en-us/library/974tc9t1(VS.80).aspx

You can use _CrtDumpMemoryLeaks to generate an error report if the application failed to free all the memory it allocated. This should be at the very end of your program.
http://msdn2.microsoft.com/en-us/library/d41t22sb(VS.80).aspx

Use can use _CrtSetBreakAlloc or _crtBreakAlloc to set break points where specific heap is allocated (as reported by CrtDumpMemoryLeaks) so that you can see where the problem starts
http://support.microsoft.com/kb/151585

Memory leak detection and isolation: http://msdn2.microsoft.com/en-us/library/x98tx3cf(VS.80).aspx

-Rx.
   
0
jkrCommented:
1st of all, there are a lot of return paths where I miss 'Release()' on active objects like

                               if(oldHBitmap == NULL) {
                                        OutputDebugString("GetCurrentObj failed\n");
                                        return true;

you might want to use scoped smart pointers instead
#import "C:\winnt\system32\mshtml.tlb" // location of mshtml.tlb
 
using namespace MSHTML;
 
        IHTMLDocument3Ptr pDocument3 = NULL;
        IHTMLDocument2Ptr pDocument  = NULL;
        IHTMLElement2Ptr pElement2   = NULL;
        IHTMLElementPtr pElement     = NULL;
        IViewObject2Ptr pViewObject  = NULL;
        IDispatchPtr pDispatch       = NULL;

Open in new window

0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
DrivenXAuthor Commented:
Thanks for the replies evilrix, jkr

I actually didnt get any memory leaks on a good run of the program.
But if i try to generate two images in quick succession i get the dump in the attached file, i will try to investigate.
Was wondering if a program that runs depending on user interaction would affect the use of _CrtSetBreakAlloc or _crtBreakAlloc.  Would the user have to do the same sequence of events, otherwise it may not be the same block of memory allocated the next time it is run?

jkr, I will look into smart pointers, do you have any recommendations on readings?  Thanks for the headsup on the return paths.

Am I doing the releasing and deleting of the DCs and the hBitmap correctly?  Im concerned about HDC hdcMain

Thanks

Dump.txt
0
Cloud Class® Course: Microsoft Office 2010

This course will introduce you to the interfaces and features of Microsoft Office 2010 Word, Excel, PowerPoint, Outlook, and Access. You will learn about the features that are shared between all products in the Office suite, as well as the new features that are product specific.

jkrCommented:
>>Was wondering if a program that runs depending on user interaction would
>>affect the use of _CrtSetBreakAlloc or _crtBreakAlloc.

Only the debug build. Anyway, these two functions won't get you any further anyway when you already have Purify. I am more acquainted with BoundsChecker, but Purify also should be able to give you more details about the allocations.
0
DrivenXAuthor Commented:
How would I go about tracking down the problems with the allocations in bounds checker?
such as in bc1.jpg in mytilus.dll, i cant seem to find anymore detail than just the .dll information
thanks
0
evilrixSenior Software Engineer (Avast)Commented:
Did you see in the links I provided that you can set break points at the point where the leaked resource is allocated? The debugger will kick in (if run with debugger enabled) at the point of allocation. You'll can then track the code path (by stepping through it for example) to see where it's going wrong, for example returning without releasing as jkr advised above. Although Purify or bounds checker can provide metrics to help track these issues down I've found, from experience, it's often necessary to follow the code to get an idea of exactly what is happening. Somethings it can be a time consuming process. To be honest, prevention is always better that cure so ubiquitous use of the RAII idiom should be adopted -- as jkr intimated, consider using smart pointers.

http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization
0
DrivenXAuthor Commented:
Thanks evilrix and jkr
Sorry if it seemed I just wanted a quick solution to this.  I know I'll have to do some work and ill set the break points after getting the smart pointers working.

I'm attempting to use smart pointers here:

      CComPtr<IHTMLDocument3> pDocument3;
        CComPtr<IHTMLDocument2> pDocument;
        CComPtr<IHTMLElement2> pElement2;
        CComPtr<IHTMLElement> pElement;
        CComPtr<IViewObject2> pViewObject;
        CComPtr<IDispatch> pDispatch;
        //IDispatch* pWebBrowserDisp = NULL;

        HRESULT hr;
        long bodyHeight;
        long bodyWidth;
        long rootHeight;
        long rootWidth;
        long height;
        long width;

        hr = m_pBrowser->get_Document(&pDispatch);

        if (FAILED(hr))
            return true;

        hr = pDispatch->QueryInterface(IID_IHTMLDocument2, (void**)&pDocument);

        if (FAILED(hr))
            return true;

        hr = pDocument->get_body(&pElement);

        if (FAILED(hr))
            return true;

        hr = pElement->QueryInterface(IID_IHTMLElement2, (void**)&pElement2);

        if (FAILED(hr))
            return true;

        hr = pElement2->get_scrollHeight(&bodyHeight);

        if (FAILED(hr))
            return true;

        hr = pElement2->get_scrollWidth(&bodyWidth);

        if (FAILED(hr))
            return true;

        hr = pDispatch->QueryInterface(IID_IHTMLDocument3, (void**)&pDocument3);

        if (FAILED(hr))
            return true;

        hr = pDocument3->get_documentElement(&pElement);   // <----------- here

        if (FAILED(hr))
            return true;

        hr = pElement->QueryInterface(IID_IHTMLElement2, (void**)&pElement2);

        if (FAILED(hr))
            return true;

but i get an ASSERT error in atlcomcli.h line 149 Expression: p==0
following the stack, it is from the line indicated in the above code where I try to reuse pElement again
should i create another IHTMLElement?
0
DrivenXAuthor Commented:
I was looking through the CreateImage function and commenting parts of the code out until I found the code which contributed to the high mem usage i see from task manager.  (something which i should have done in the beginning..) and it turns out to be the following:

        CRect rect(CPoint(0, 0), srcSize);
 
        //      The WebBrowswer window size must be set to our srcSize
        //      else it won't render everything
        MoveWindow(&rect);
        m_pBrowserWnd.MoveWindow(&rect);
 
        COleVariant       vUrl(szSrcFilename, VT_BSTR),
                                  vFlags(long(navNoHistory | navNoReadFromCache | navNoWriteToCache), VT_I4),
                                  vNull(LPCTSTR(NULL), VT_BSTR);
        COleSafeArray vPostData;
 
        if (m_pBrowser->Navigate2(&vUrl, &vFlags, &vNull, &vPostData, &vNull) == S_OK)
                //      We have to pump messages to ensure the event handler (DocumentComplete)
                //      is called.
                RunModalLoop();
        else
                return FALSE;


where CComPtr<IWebBrowser2> m_pBrowser
RunModalLoop() is the line that causes the ~70,000K increase in mem usage
what exactly happens inside this function? Is there anyway to free the resources it used after the loop ends?
0
DrivenXAuthor Commented:
The memory leak was coming from two dlls, provided by mcafee (mytilus.dll) and itunes bonjour service (mdnsnps.dll).  After unregistering these 2 dlls and some tweaking of the code with suggestions from jkr and evilrix, i think the mem usage is acceptable.  Thanks
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
System Programming

From novice to tech pro — start learning today.