Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

MFC and PostMessage problem in Release build

157 views
Skip to first unread message

Faisal

unread,
Feb 6, 2009, 12:10:40 AM2/6/09
to
I 've an application developed with MFC, which behaves abnormally in
the release build.
It's a dialog-based application with two buttons START and STOP. While
clicking the START, it starts a thread. The thread just sleeps 2000ms
and posts a user-defined message to the dialog. In this message
handler the START button is enabled and STOP is disabled.

My implementation is given below.


#define UWM_STOP (WM_USER + 124)

BEGIN_MESSAGE_MAP(CPostTestDlg, CDialog)
//{{AFX_MSG_MAP(CPostTestDlg)
ON_BN_CLICKED(IDC_BTN_START, OnBtnStart)
ON_BN_CLICKED(IDC_BTN_STOP, OnBtnStop)
ON_MESSAGE(UWM_STOP, OnBtnStop )
//}}AFX_MSG_MAP
END_MESSAGE_MAP()

DWORD WINAPI CPostTestDlg::ThreadFn( LPVOID lpParam )
{
CPostTestDlg* pThis = static_cast<CPostTestDlg*>(lpParam);

while( 1 )
{
Sleep( 2000 );

TRACE(_T("Posting message\n"));
::PostMessage( pThis->m_hWnd, UWM_STOP, 0, 0 );
break;
}

return 0;
}


void CPostTestDlg::OnBtnStart()
{
GetDlgItem( IDC_BTN_START )->EnableWindow( FALSE );
GetDlgItem( IDC_BTN_STOP )->EnableWindow( TRUE );


DWORD dwThreadID;
m_hThread = ::CreateThread(NULL, 0, ThreadFn, (void*)this, NULL,
&dwThreadID);
}

void CPostTestDlg::OnBtnStop()
{
TRACE(_T("Stop receieved\n"));

::WaitForSingleObject( m_hThread, INFINITE );
::CloseHandle( m_hThread );

GetDlgItem( IDC_BTN_START )->EnableWindow( TRUE );
GetDlgItem( IDC_BTN_STOP )->EnableWindow( FALSE );

}

These code runs smoothly in debug build. When I run it in release
mode, for the first two START and STOP it works fine. When I click
the start third time it crashes.

I've encountered this problem in a complex project. In order to
confirm this issue, I tried this in a sample project (code listed
above). But this problem occurs in this simple project too.

Could someone see something wrong in my code?

Doug Harrison [MVP]

unread,
Feb 6, 2009, 12:28:38 AM2/6/09
to
On Thu, 5 Feb 2009 21:10:40 -0800 (PST), Faisal <fais...@gmail.com>
wrote:

>I 've an application developed with MFC, which behaves abnormally in
>the release build.
>It's a dialog-based application with two buttons START and STOP. While
>clicking the START, it starts a thread. The thread just sleeps 2000ms
>and posts a user-defined message to the dialog. In this message
>handler the START button is enabled and STOP is disabled.
>
>My implementation is given below.
>
>
>#define UWM_STOP (WM_USER + 124)
>
>BEGIN_MESSAGE_MAP(CPostTestDlg, CDialog)
> //{{AFX_MSG_MAP(CPostTestDlg)
> ON_BN_CLICKED(IDC_BTN_START, OnBtnStart)
> ON_BN_CLICKED(IDC_BTN_STOP, OnBtnStop)
> ON_MESSAGE(UWM_STOP, OnBtnStop )
> //}}AFX_MSG_MAP
>END_MESSAGE_MAP()

You should move your ON_MESSAGE after the AFX comments, but you can't use
the same handler function in ON_BN_CLICKED and ON_MESSAGE. See below for
more.

You're using the same message handler for ON_BN_CLICKED and ON_MESSAGE,
whose handler functions are totally different. The ON_MESSAGE handler is
declared like this:

LRESULT Handler(WPARAM, LPARAM);

Also, you're using the raw API to manage your threads, when you should be
using AfxBeginThread. See this page for some very important info on using
MFC's CWinThread correctly:

http://members.cox.net/doug_web/threads.htm

--
Doug Harrison
Visual C++ MVP

Faisal

unread,
Feb 6, 2009, 12:53:33 AM2/6/09
to
On Feb 6, 10:28 am, "Doug Harrison [MVP]" <d...@mvps.org> wrote:
> On Thu, 5 Feb 2009 21:10:40 -0800 (PST), Faisal <faisal...@gmail.com>

> wrote:
>
>
>
> >I 've an application developed with MFC, which behaves abnormally in
> >the release build.
> >It's a dialog-based application with two buttons START and STOP. While
> >clicking the START, it starts a thread. The thread just sleeps 2000ms
> >and posts a user-defined message to the dialog. In this message
> >handler the START button is enabled and STOP is disabled.
>
> >My implementation is given below.
>
> >#define UWM_STOP (WM_USER + 124)
>
> >BEGIN_MESSAGE_MAP(CPostTestDlg, CDialog)
> >    //{{AFX_MSG_MAP(CPostTestDlg)
> >    ON_BN_CLICKED(IDC_BTN_START, OnBtnStart)
> >    ON_BN_CLICKED(IDC_BTN_STOP, OnBtnStop)
> >    ON_MESSAGE(UWM_STOP, OnBtnStop )
> >    //}}AFX_MSG_MAP
> >END_MESSAGE_MAP()
>
> You should move your ON_MESSAGE after the AFX comments,

Is this a coding practice? Or necessary?

Thanks Doug.
The type of Handler was the problem.

Joseph M. Newcomer

unread,
Feb 6, 2009, 3:28:32 PM2/6/09
to

On Thu, 5 Feb 2009 21:10:40 -0800 (PST), Faisal <fais...@gmail.com> wrote:

>I 've an application developed with MFC, which behaves abnormally in
>the release build.
>It's a dialog-based application with two buttons START and STOP. While
>clicking the START, it starts a thread. The thread just sleeps 2000ms
>and posts a user-defined message to the dialog. In this message
>handler the START button is enabled and STOP is disabled.
>
>My implementation is given below.
>
>
>#define UWM_STOP (WM_USER + 124)

****
DO NOT use WM_USER-based messages! This is an application message, and you should either
use WM_APP-based messages, or better still, Registered Window Messages. But WM_USER is
limited to controls, and you are not sending this message to a control
****


>
>BEGIN_MESSAGE_MAP(CPostTestDlg, CDialog)
> //{{AFX_MSG_MAP(CPostTestDlg)
> ON_BN_CLICKED(IDC_BTN_START, OnBtnStart)
> ON_BN_CLICKED(IDC_BTN_STOP, OnBtnStop)
> ON_MESSAGE(UWM_STOP, OnBtnStop )

****
This is the wrong thing to do. It SHOULD crash. Did you bother to read the
documentation, which CLEARLY states that an ON_MESSAGE handler has the prototype
afx_msg LRESULT (CWnd::*)(WPARAM, LPARAM);
which means you must declare it as
afx_msg LRESULT handlername(WPARAM, LPARAM)
in your window class.

Why would you think that you could substitute a function that has a prototype
void handlername()
in its place?

I notice you did not mention whether you were using VS6, VS2002, VS2003, VS2005 or VS2008.
How are we supposed to analyze problems when we don't even know what compiler you are
using? However, this bug only occurs in VS6 because the later versions refuse to compile
code this ridiculous.

When in doubt, try reading the documentation.

In any case, if you did
LRESULT OnStopMsg(WPARAM, LPARAM)
{
OnBtnStop();
return 0;
}
it would still be incorrect, because OnBtnStop responds to user input to REQUEST the
thread stop, and you want your handler to respond to the fact that your handler HAS
stopped! These are totally different tasks (and if you don't think they are, you need to
arrive at that understanding before proceeding further).
****


> //}}AFX_MSG_MAP
>END_MESSAGE_MAP()
>
>DWORD WINAPI CPostTestDlg::ThreadFn( LPVOID lpParam )
>{
> CPostTestDlg* pThis = static_cast<CPostTestDlg*>(lpParam);
>
> while( 1 )
> {
> Sleep( 2000 );
>
> TRACE(_T("Posting message\n"));
> ::PostMessage( pThis->m_hWnd, UWM_STOP, 0, 0 );

****
Why write something so complex? What's wrong with the much simpler
pThis->PostMessage(UWM_STOP);
What part of "Postmessage is a member of the CWnd class" did you fail to understand???
****


> break;
> }
>
> return 0;
>}
>
>
>void CPostTestDlg::OnBtnStart()
>{
> GetDlgItem( IDC_BTN_START )->EnableWindow( FALSE );
> GetDlgItem( IDC_BTN_STOP )->EnableWindow( TRUE );

****
Don't use GetDlgItem. Learn to create member variables. This MFC, not native Win32 code.
See my essay on Avoding GetDlgItem on my MVP Tips site.

I note that if the thread creation fails, you have the controls disabled with no way to
enable them. They should not be disabled until you know you have a running thread.
****


>
>
> DWORD dwThreadID;
> m_hThread = ::CreateThread(NULL, 0, ThreadFn, (void*)this, NULL,
>&dwThreadID);

****
This is completely wrong. The ONLY correct way to start a thread in MFC is to use
AfxBeginThread. ::CreateThread will create a thread that is not MFC-compatible and does
not have the necessary context initialized. Do NOT use ::CreateThread in MFC apps! In
fact, do not use it at all, ever. If you use it, you cannot use MFC or the C runtimes in
the thread. There are some extremely rare and exotic circumstances in which CreateThread
can be used, but none of them exist in an MFC program.
****


>}
>
>void CPostTestDlg::OnBtnStop()
>{
> TRACE(_T("Stop receieved\n"));

****
I see nothing here that would cause the thread to stop. So why do you think it will? You
don't need a Stop button if the thread stops itself upon completion; you only need a Stop
button to abort control.
****
>
> ::WaitForSingleObject( m_hThread, INFINITE );
****
Either the thread shuts itself down, in which case you don't need the WFSO, or you have to
shut it down, in which case you don't need the WFSO.

I think this arises because you have confused a thread stop REQUEST with a thread stop
NOTIFICATION. They are not the same, and cannot be handled in the same function!
****
> ::CloseHandle( m_hThread );
****
You didn't need the thread handle in the first place, so why wait until now to close it?
*****


>
> GetDlgItem( IDC_BTN_START )->EnableWindow( TRUE );
> GetDlgItem( IDC_BTN_STOP )->EnableWindow( FALSE );

****
What's with this "GetDlgItem" stuff? Create member variables, and forget you ever heard
about GetDlgItem.
c_Start.EnableWindow(TRUE);
c_Stop.EnableWindow(FALSE);

Also, it is generally bad practice to enable and disable controls all over your dialog;
create a single function that handles this. See my essay on dialog control management on
my MVP Tips site.
****


>
>}
>
>These code runs smoothly in debug build. When I run it in release
>mode, for the first two START and STOP it works fine. When I click
>the start third time it crashes.

****
Crashes? I suppose you have some description about what you mean by this nonsense word?
There is not such thing as a "crash" unless your question includes the EXACT DESCRIPTION
OF WHAT HAPPENED!!!! Given the non-information you have given, the answer is going to be
one of "You failed to read the documentation and wrote code that cannot possibly work
because it is nonsensical" (not using the correct prototype for a handler function), or
"You used ::CreateThread erroneously, replace it with the only correct way to create a
thread in MFC, AfxBeginThread" or "Something else is wrong, fix it". In the latter case,
there's no way to tell what that is because you gave a content-free description of the
problem.

DId you use the debugger to determine what thread caused the problem? Did you look at the
call stack? What did you learn, and why did you not tell us what you have discovered?
****


>
>I've encountered this problem in a complex project. In order to
>confirm this issue, I tried this in a sample project (code listed
>above). But this problem occurs in this simple project too.

****
Then the complex project is as badly broken as this toy example.

In a program that has only 30 lines, I count eight problems.

Four of them are fatal design problems
- Nonsensical ON_MESSAGE handler function that violates the specification
- Confusing user the stop request with thread stop notification
- Using CreateThread instead of AfxBeginThread
- Disabling controls before any known way to re-enable them exists

These have to be fixed before you can do anything else.

You should also fix the serious coding problems
- Using GetDlgItem
- Using ::PostMessage when it is not needed (CWnd::PostMessage should be used)
- Enabling/disabling controls in a distributed fashion
- Using a window-local basis of messages for an application message
joe
****


>
>Could someone see something wrong in my code?

Joseph M. Newcomer [MVP]
email: newc...@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm

0 new messages