Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 1772
  • Last Modified:

MFC: SetIcon() problem if called twice

Dear experts,

I have an MFC application. In one of the dialogs, there is a button. I set an icon for this button in OnInitDialog() like this:
      if(some condition)
      {
            m_btnBold.SetIcon(static_cast<HICON>(::LoadImage(AfxGetInstanceHandle(), MAKEINTRESOURCE(IDI_BOLD_PRESSED), IMAGE_ICON, 0, 0, LR_DEFAULTCOLOR)));
      }
      else
      {
            m_btnBold.SetIcon(static_cast<HICON>(::LoadImage(AfxGetInstanceHandle(), MAKEINTRESOURCE(IDI_BOLD), IMAGE_ICON, 0, 0, LR_DEFAULTCOLOR)));
      }

But also this code is called in the method that handles certain event in the dialog.
This causes a crash with a break here:
_CRTIMP void _cdecl _CrtDbgBreak(
    void
    )
{
    DebugBreak();
}
There is an error message in the debug output window:
Second Chance Assertion Failed: File f:\dd\vctools\vc7libs\ship\atlmfc\include\afxwin1.inl, Line 395
Book.exe has triggered a breakpoint

If I call SetIcon code only once, everything works fine. I guess I am missing some cleanup, but what and where? Could you please help me with that?
Thanks.
P.S. The crash happens not when SetIcon() is executed, but when I press OK and exit the dialog and the method from where the dialog's DoModal() is called. Also, it happens not the first time, but exactly the fifth opening of the dialog.
0
tantormedia
Asked:
tantormedia
  • 10
  • 7
  • 4
  • +1
1 Solution
 
jkrCommented:
What exactly is at

f:\dd\vctools\vc7libs\ship\atlmfc\include\afxwin1.inl, Line 395

?
0
 
tantormediaAuthor Commented:
I have no idea. I don't even know what f:\ is.
0
 
jkrCommented:
Are you linking with a 3rd party library?
0
Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
tantormediaAuthor Commented:
Not for this functionality.
0
 
jkrCommented:
Interesting - can you post the call stack when this happens?
0
 
tantormediaAuthor Commented:
It is not much:
       ntdll.dll!7c90120e()       
>      Book.exe!_CrtDbgBreak()  Line 89      C


The bottom line is where the break happens:
_CRTIMP void _cdecl _CrtDbgBreak(
    void
    )
{
    DebugBreak();
}

But as I said, it must have something to do with multiple calls to SetIcon() or to LoadImage() in the same line, as when I comment out the second call, the problem is gone.
0
 
jkrCommented:
You should download and install the symbol package that matches your OS and SP version, so that we know what ' ntdll.dll!7c90120e()' in fact is - see http://msdn.microsoft.com/en-us/windows/hardware/gg463028.aspx#d ("Download Windows Symbol Packages")
0
 
JimBeveridgeCommented:
What you are seeing is probably a standard assertion in MFC. If you run this in the debugger in Visual Studio, the debugger should break on the correct line and allow you to see what's happening.

If this doesn't work, then see my blog on this problem:
http://qualapps.blogspot.com/2007/04/symbols-for-mfc-source-code.html

Alternatively, you can just manually look at that line in the MFC source code. That file is normally found at:
C:\Program Files\Microsoft Visual Studio 9.0\VC\atlmfc\include\afxwin1.inl

jkr's last comment about downloading the symbol package is a bad idea for three reasons:
1. The function in ntdll is simply DebugBreak. You don't need any symbols to figure that out.
2. This is an MFC problem and the MFC symbols are part of Visual Studio, not part of the OS symbol package.
3. The OS symbol packages are out of date a week after they ship. Instead, you should configure Visual Studio to automatically download symbols as needed.
0
 
ZoppoCommented:
Hi tantormedia,

>> But also this code is called in the method that handles certain event in the dialog.
Could you tell us what events are handled which cause the posted function to be called? Maybe for any reason this is called after the dialog was closed or the button was destroyed or something like this.

Depending on the VisualStudio version you use CButton::SetIcon is implemented different, but it should look somehow like this:
_AFXWIN_INLINE HICON CButton::SetIcon(HICON hIcon)
	{ ASSERT(::IsWindow(m_hWnd)); return (HICON)::SendMessage(m_hWnd, BM_SETIMAGE, IMAGE_ICON, (LPARAM)hIcon); }

Open in new window

So, IMO the only cause for an ASSERT can occur if the button's 'm_hWnd' isn't a valid window handle.

ZOPPO
0
 
tantormediaAuthor Commented:
Thank you all who are trying to help.

JimBeveridge, I opened that file, and to my surprise, now the debugger shows the problematic line (I wonder why the VS didn't open it itself).

Here is the call stack:

>      Book.exe!CDC::SelectObject(CFont * pFont=0x0012fad8)  Line 261 + 0x1c bytes      C++
       Book.exe!CBookView::OnMouseMove(unsigned int nFlags=0, CPoint point={...})  Line 2241 + 0x1c bytes      C++

And the line is:
CFont* CDC::SelectObject(CFont* pFont)
{
>      ASSERT(m_hDC != NULL);
...

Could you please suggest why this happens and how it can be fixed?

Thanks.

0
 
tantormediaAuthor Commented:
I think I have to add something. I don't think the problem I described in my previous post was the initial one. As it had something to do with mouse moves, I tried to avoid moving the mouse at all, and thus I could recreate the problem we talked about before. It is here:
AFXWIN_INLINE CBitmap* CDC::SelectObject(CBitmap* pBitmap)
      { ASSERT(m_hDC != NULL); return (CBitmap*) SelectGdiObject(m_hDC, pBitmap->GetSafeHandle()); }
So, m_hDC is NULL. Could you tell, why?
Thanks.
P.S. The call stack looks like this:
       ntdll.dll!7c90120e()       
       [Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]      
>      Book.exe!_CrtDbgBreak()  Line 89      
0
 
ZoppoCommented:
Hi,

could you post the code of your CBookView::OnMouseMove where you use 'SelectObject'?

Generally you cannot call 'SelectObject' against a CDC which isn't valid (thus a CDC either passed to OnDraw or a DC created i.e. with CClientDC constructor).

ZOPPO
0
 
tantormediaAuthor Commented:
Zoppo, as I wrote in my last post, I tried to exclude MouseMove event at all, at least so far.

Thanks.
0
 
ZoppoCommented:
Well, I guess in general it doesn't matter - wherever you recieve one of these assertions ASSERT( m_hDC != NULL ); you are using CDC function 'SelectObject' in an incorrect way - so I asked for OnMouseMove since I don't know in which function you use the 'CDC::SelectObject(CBitmap* pBitmap)' you mentioned in your previous comment.

Anyhow - please check the stack in which of your functions the debugger halts when the ASSERT occurs - i.e. for the case of 'OnMouseMove' the interesting line in the stack should look like:
Book.exe!CBookView::OnMouseMove

Open in new window

In the other case it probably halted in another function.

Please post one of these functions to let us see how the 'SelectObject' is used there ...

ZOPPO
0
 
tantormediaAuthor Commented:
I seems to me that that SelectObject() problem only indicates the real problem that is connected with this code:
      if((format.dwEffects & CFE_BOLD) == CFE_BOLD)
      {
            m_btnBold.SetIcon(static_cast<HICON>(::LoadImage(AfxGetInstanceHandle(), MAKEINTRESOURCE(IDI_BOLD_PRESSED), IMAGE_ICON, 0, 0, LR_DEFAULTCOLOR)));
      }
      else
      {
            m_btnBold.SetIcon(static_cast<HICON>(::LoadImage(AfxGetInstanceHandle(), MAKEINTRESOURCE(IDI_BOLD), IMAGE_ICON, 0, 0, LR_DEFAULTCOLOR)));
      }

because without calling it multiple times, no problem happens. Don't you think that there is some resource leak? Maybe I should release DC explicitly, or something like that?
This code is called from a modal dialog events handlers, and all cases of calling SelectObject() are connected to the view.
0
 
ZoppoCommented:
Well, sorry, but I think it's difficult to find the reason this way - couldn't you post some more info and/or code? The code you posted now for sure cannot lead to a ASSERTion in the previously posted 'SelectObject' - 'SetIcon' IMO can only ASSERT in that last code if the 'm_hWnd' of 'm_btnBold' isn't valid anymore.

Therefore I asked you some comments above to '... tell us what events are handled which cause the posted function to be called?'.

If the first time 'm_btnBold.SetIcon' is called without any ASSERTion but ASSERTs when called another time the 'm_btnBold' must have become invalid i.e. due to destruction of it's parent dialog or something else ...

0
 
tantormediaAuthor Commented:
OK, here is the method you asked about:

void CBookView::OnMouseMove(UINT nFlags, CPoint point)
{
	CBookDoc* pDoc = GetDocument();

	if(!pDoc->m_pCurChapter)
	{
		return;
	}
	CPoint ptScrollOff = GetScrollPosition();
	CPoint OffPoint = point + CSize(ptScrollOff.x, ptScrollOff.y);
	
	TEXTMETRIC tm;
	CFont TextFont;

	CBookApp* pApp = (CBookApp * ) AfxGetApp();
	TextFont.CreateFontIndirect(&pApp->m_Font);

	HDC hDC = CreateCompatibleDC(NULL);
	CDC* pDC = new CDC;
	pDC->Attach(hDC);
	pDC->SelectObject(&TextFont);

Open in new window

0
 
ZoppoCommented:
ok - thanks - do you even release the selected font and delete the DC using 'DeleteDC' or 'delete pDC'? If not I guess your real problem is GDI leaking. There's a limited number of resources (i.e. fonts, bitmaps) and DCs which can be created at one time. If now you i.e. create a new DC with every mouse move I guess this leaks very fast.

Further 'DeleteDC' probably cannot release the created DC if the font which was selected into it wasn't de-selected again.

So if this code is just a little wrong or maybe mis-designed depends a bit on what you're doing after it. To use a memory DC (which is created by 'CreateCompatibleDC') you also have to select a bitmap into it. For both objects you select into the memory DC you need to store the previously selected object returned by 'SelectObject' to be able to later re-select them.

Maybe you could explain what you want to achieve with your code at all?
0
 
tantormediaAuthor Commented:
Yes, I do:
      pDC->SelectObject(&TextFont);
      pDC->GetTextMetrics(&tm);
      delete pDC;

But again, the problem must be connected to multiple calls to that SetIcon() thing. Without it, no problem happens. It seems like that is what exhausts the resources. But as I don't use DC explicitly, I don't know how to release it. :(
0
 
ZoppoCommented:
Well, that's not sure - IMO at least the OnMouseMove code produces a resource leak since the font isn't deselected.

Change that fragment to be like this:
      CFont* pOldFont = pDC->SelectObject(&TextFont);
      pDC->GetTextMetrics(&tm);
      pDC->SelectObject( pOldFont );
      delete pDC;

Open in new window


What's about that icons is hard to say too without seeing more code - anyhow I guess that these are leaking too since everytime you call 'LoadImage' which creates a new icon resource in memory but you don't free them.

Generally I think it's a better practice to load the icons once (i.e. on application startup or when the view is created) and store them as members - then in your function you can re-use these members with 'SetIcon'; and later on you should destroy the icons again (i.e. on application termination or on view destruction).


It's still hard to say why 'SetIcon' ASSERTs - I still think the 'm_btnBold.m_hWnd' isn't valid anymore. Just to verify this you could add this line before the 'if':
ASSERT( NULL != m_btnBold.m_hWnd && FALSE != ::IsWindow( m_btnBold.m_hWnd ) );

Open in new window


If I'm right than this ASSERTion should occur. In this case you have to find out when/why the button's control is destroyed. It might be it's a side-effect from the resource leaks - if the system is out of free resources anything can happen, hard to predict ...

ZOPPO
0
 
tantormediaAuthor Commented:
The first change that you suggested didn't work. But when I stored icon handles in application member variables on InitInstance() as you suggested, the problem is solved!

Thank you very much.
0
 
ZoppoCommented:
Great - you're welcome - I'm glad I could help ...

have a nice day,

best regards,

ZOPPO
0

Featured Post

Free Tool: ZipGrep

ZipGrep is a utility that can list and search zip (.war, .ear, .jar, etc) archives for text patterns, without the need to extract the archive's contents.

One of a set of tools we're offering as a way to say thank you for being a part of the community.

  • 10
  • 7
  • 4
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now