[Webinar] Streamline your web hosting managementRegister Today

x
• Status: Solved
• Priority: Medium
• Security: Public
• Views: 1817

# 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
• 10
• 7
• 4
• +1
1 Solution

Commented:
What exactly is at

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

?
0

Author Commented:
I have no idea. I don't even know what f:\ is.
0

Commented:
Are you linking with a 3rd party library?
0

Author Commented:
Not for this functionality.
0

Commented:
Interesting - can you post the call stack when this happens?
0

Author 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

Commented:
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

Commented:
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

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

Commented:
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); }

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

ZOPPO
0

Author 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

Author 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

Commented:
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

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

Thanks.
0

Commented:
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

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

Author 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

Commented:
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

Author Commented:

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);

0

Commented:
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

Author 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

Commented:
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;


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 ) );


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

Author 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

Commented:
Great - you're welcome - I'm glad I could help ...

have a nice day,

best regards,

ZOPPO
0

## Featured Post

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