Re: Bug in win xp: Auto-detaching modeless dialog.

From: Alexander Grigoriev (alegr_at_earthlink.net)
Date: 10/23/04 

Date: Fri, 22 Oct 2004 21:28:18 -0700

OK, you forgot to mention, that if a window doesn't respond to messages for
some long time, it will be overlayed by a "ghost" window, with white
background. This "ghost" will allow to kill the runaway task.
But if an application runs under debugger, "ghost" windows are not created.
This is why the behavior is different under VS.

"Joseph M. Newcomer" <newcomer@flounder.com> wrote in message
news:n9bin0ptqmutlqect2hb4nn0q9l7vop7on@4ax.com...
> Many things wrong with the code here. Like most of it.
>
> It is usually a good idea to use classname::IDD for the ID of the dialog
> box, rather than
> the explicit ID. While they are the same, the classname::IDD (for
> classname the name of
> your class) is easier to write and read.
>
> You should not make the variable a CDialog*, but the appropriate type for
> your dialog!
> This failure can lead to all sorts of catastrophes as you overwrite
> memory.
>
> CMyDialog * prog = new CMyDialog;
> BOOL ret = prog->Create(CMyDialog::IDD);
>
> would be the tasteful way to create the box.
>
> Avoid silly beginner mistakes like using a symbol like "I" (or is it "l"?)
> to name an
> identifier. That is, is it lower-case L or capital I? HN is bad enough
> without things like
> this contributing to its unreadability.
>
> if(!ret) AfxMessageBox(...);
>
> works but what happens next? If it failed, why aren't you returning?
> Instead, you go and a
> process as if it had worked anyway! And I note that you do not actually
> delete the CDialog
> object you created; this leads to a storage leak.
>
> What are you doing declaring a char buffer and doing itoa? This is silly C
> programming.
> Use a CString and CString::Format. Writing a char buffer is usually a
> suspect programming
> practice. For one thing, it should have been TCHAR and you would have used
> the
> Unicode-aware functions to manage it, but the whole approach is just
> wrong.
>
> If you believe itoa is "more efficient" than using CString::Format, you
> need to rethink
> your approach to programming. You are saving a few hundred instructions by
> uisng itoa and
> then executing a few hundred thousand to perhaps a few million with that
> SetWindowText
> call, so what, exactly is "efficient" about saving 0.0000001% of the total
> execution of
> your program by using antiquated and error-prone methodologies?
>
> The whole loop with a sleep in it is a colossal blunder. You have tied up
> the message pump
> so the dialog is dead, dead, dead; the GUI is blocked, etc.
>
> I don't even know what "detatch" means, since there is no technical
> description of the
> word "detach" in the Windows API with respect to windows. But with a
> deeply flawed
> structure like you show here, any malfunction dealing with the GUI is not
> even worthy of
> discussion. You have such a fundamental design flaw that nothing else
> matters. Fix the
> problem of sleeping in a loop that blocks the GUI first, and then you have
> something worth
> talking about.
>
> For example, the correct approach would be to put the modeless dialog
> variable as a
> member variable of the CDialogTestView class, and create the dialog, and
> go away, having
> set a timer:
>
> void CDialgoTestView::OnTest()
> {
> if(prog != NULL)
> return; // we should not be here if a dialog is already up
> prog = new CWhateverDialog;
> if(!prog->Create(CWhateverDialog::IDD))
> {
> delete prog;
> prog = NULL;
> return;
> }
> StepsCount = MAX_STEPS;
> SetTimer(IDT_MY_TIMER, 100);
> }
>
> void CDialogTestView::OnTimer(UINT nID, ...)
> {
> CString s;
> switch(nID)
> { /* nID */
> case IDG_MY_TIMER:
> s.Format(_T("%d"), StepsCount);
> if(prog != NULL)
> prog->SetWindowText(s);
> StepsCount--;
> if(StepsCount == 0)
> { /* done */
> KillTimer(IDT_MY_TIMER);
> prog->DestroyWindow();
> prog = NULL;
> } /* done */
> break;
> ... other timer events here
> } /* nID */
> return CDialog::OnTimer(...);
> }
>
> Note that you need to make sure your modeless dialog has a PostNcDestroy
> handler that is
> like this:
>
> void CMyDialog::PostNcDestroy()
> {
> delete this;
> }
>
> which is one of the many reasons that creating a variable of type CDialog
> makes no sense.
>
> You have to stop thinking like a console app programmer and start thinking
> like a Windows
> programmer. Be very, very suspicious every time you think about writing a
> Sleep(). 90% of
> the time I see a Sleep() it is simply a fundamental design flaw. Long
> loops that block
> the message pump but interact with the user are also suspect. Either make
> them threads, or
> drive them from events that don't block the message pump. Asynchronous,
> event-driven
> programming is the paradigm of choice. You are using synchronous,
> procedural programming,
> and it just doesn't fit the Windows model.
> joe
>
>
>
> On 22 Oct 2004 02:12:37 -0700, ril@op.pl (Ril) wrote:
>
>>Hi.
>>If I create a modeless dialog box in win xp and in the same method I
>>do some calculations before deleting this dialog then after about 5
>>seconds this dialog becomes an independent window and doesn't receive
>>messages from the application. This bug happens only when I start the
>>project outside the developer environment. I also have to either click
>>on the dialog box or cover part of my application window by another
>>window. This bug occured on all computers with win xp I had access to.
>>Does anybody know how I can fix it?
>>
>>Here are the steps to reproduce the error. You can download the
>>project from http://republika.pl/pointer/dialogtest.zip
>>
>>1. Create an MFC application by MFC application.
>>2. Create a method and fill it with the following code:
>>
>>void CDialogTestView::OnTest()
>>{
>> const int c_nStepsCount = 150;
>>
>> CDialog* l_Prog = new CDialog();
>>
>> //BOOL ret = l_Prog->Create(IDD_ABOUTBOX, NULL); // does not care if
>>there is NULL or this
>> BOOL ret = l_Prog->Create(IDD_ABOUTBOX, this);
>> if(!ret) AfxMessageBox("Error creating Dialog");
>>
>> l_Prog->ShowWindow( SW_SHOW );
>>
>> for (int i=0; i<c_nStepsCount; i++) {
>> Sleep(100); // way of sleeping is unimportant
>> //ActiveSleep(100); // way of sleeping is unimportant
>> char buffer[100];
>> itoa(i, buffer, 10);
>> l_Prog->SetWindowText(buffer); // The dialog detaches also when this
>>line is removed
>>
>> }
>> l_Prog->DestroyWindow();
>>}
>>
>>Active sleep procedure code is the following. It doesn't care if you
>>sleep actively or not:
>>
>>void ActiveSleep(int milisec) {
>> _timeb now, stop;
>>
>> _ftime( &stop );
>> stop.millitm += milisec;
>> stop.time += stop.millitm/1000;
>> stop.millitm = stop.millitm % 1000;
>>
>> do {
>> _ftime( &now);
>> } while (now.time < stop.time || (now.time == stop.time &&
>>now.millitm < stop.millitm));
>>}
>>
>>Now execute the program _outside_ the environment (either by Ctrl-F5
>>or executing the binary). Wait until the number on the dialog box is
>>>55 and click it. The dialog will detach, the numbers will stop
>>changing and you will be able to move this dialog box.
>>
>>Thanks in advance
>>Ril
>>ril@op.pl
>
> Joseph M. Newcomer [MVP]
> email: newcomer@flounder.com
> Web: http://www.flounder.com
> MVP Tips: http://www.flounder.com/mvp_tips.htm