ListView problem when shutdown the app

[Frunktz]

Hi @ all!
I have a little, but infamous problem...
My program is structured like this.


void CMyDlg::OnRefreshMessage()
{
ListView1.InsertItem(0, "aaa");

if (bProgramShutdown)
ListView1.InsertItem(0, "Closing...");
}

void CMyDlg::OnWaitForClose()
{
WaitForSingleObject(m_hThread, INFINITE);
bCanClose = true;
PostMessage(WM_CLOSE);
}

void ThreadProc()
{
... some stuff like --- while (bActive)
PostMessage(hWnd, REFRESH_MEX, 0, 0);
}

void CMyDlg::OnClose()
{
if (!bCanClose)
{
bProgramShutdown = true;
PostMessage(hWnd, WAIT_FOR_CLOSE, 0, 0);
PostMessage(hWnd, WAIT_FOR_CLOSE, 0, 0);
}
}


Well the problem.
If I close my app, the text "Closing..." don't appear on the ListView...Why this???

[MikeAThon]

Hi @ all!
I have a little, but infamous problem...
...Well the problem.
If I close my app, the text "Closing..." don't appear on the ListView...Why this???

It's a lot of code to digest, and I didn't follow it all.

But I think one cause for your "problem" is that you are posting the WAIT_FOR_CLOSE message rather than sending it. PostMessage adds the message to the end of the queue; but since your app is already processing the WM_CLOSE message, it will never retrun to the queue to process the newly-posted WAIT_FOR_CLOSE message. You might try a SendMessage(.., WAIT_FOR_CLOSE, ...) instead.

Regards,
Mike

PS: In the future, please enclose your code in [ code ] and [ / code ] tags; your code is hard to read without them.

[Frunktz]

Sorry for the TAG code, in next 3d, I will insert that.
Well, if I use SendMessage, there no changes...Why?
Last, the code that I had post, it's sufficient for understand the problem, I suppose, infact I try to comment the other code from my app and the result it's the same.

[MrViggy]

Are you sure the text is not appearing, and you are not seeing it because the app is closing? Is "ListView" part of the dialog that you are closing?

Also, are you giving the app enough time to repaint itself? I'm thinking that if you change the message, then send yourself a close message, your dialog is not getting enough time to repaint itself, because it's closing.

Viggy

[Arjay]

Can you post a zipped code sample (sans .ncb and build directories)?

Arjay

[Frunktz]

Uhm, mayba it can be the time. But I think 2 second or more, waiting the thread stopping, it's enought...
Now, I prepare a zip file, and I post it.

[MrViggy]

Oh, wait! In the funtion 'CMyDlg::OnWaitForClose()', you are suspending the GUI thread, waiting on the worker thread. This means that your GUI will not refresh itself.

Instead, you should send your GUI a message from the thread that the thread is done. Otherwise, you'll have to pump your GUI's messages while waiting for the thread to complete. I haven't tried this, but this might work:
void CMyDlg::OnWaitForClose()
{
while (WaitForSingleObject(m_hThread, 0) != WAIT_OBJECT_0)
{
MSG msg;
while( ::PeekMessage( &msg, NULL, 0, 0, PM_REMOVE ) )
{
::TranslateMessage( &msg );
::DispatchMessage( &msg );
}
}
bCanClose = true;
PostMessage(WM_CLOSE);
}
You may need to add some checks to see if m_hThread is valid, and running before doing the WFSO call.

Viggy

[Frunktz]

Wow, great!
Now the problem is disappear. And the GUI isn't freeze. Thanks a lot.
Last question, another way to avoid the "GUI freeze", can I start another thread like this


UINT ThreadWaitForClose(LPVOID)
{
WaitForSingleObject(m_hThread, INFINITE);
return(0);
}


Maybe it's too dispendius create another thread to check if the other thread has finish..
Maybe...
Thanks again

[Arjay]

I didn't get a chance to reply yesterday... so starting at the beginning of the thread.

Uhm, mayba it can be the time. But I think 2 second or more, waiting the thread stopping, it's enought...
In general thread syncronization shouldn't be performed by any specific time amounts. In other words you shouldn't wait for a certain amount of time for something to happen; otherwise, you'll run into problems when running your app on different machines with different processor speeds and memory capacities. What works fine on your dev machine may not work on a slower machine. Instead use events to manage your thread.

I see multiple problems with the application, but mainly:
1) The thread proc has no way to know the app is terminating and when it should exit
2) OnWaitForCloseMex() is hanging (that's why your app doesn't close immediately). This is because you are calling WFSO(hThread, INFINITE). This blocks the dialog from closing until the thread has finish its while loop. To see this, change the while( i < 10 ) statement to while( i < 100 ). Notice it now takes longer for the app to close.

Last question, another way to avoid the "GUI freeze", can I start another thread like thisI don't believe this is necessary. In fact, quite often I see folks using messages to help control threads when they should be using events (IMO).

I haven't tried this, but this might work:MrViggy, rather than using WFSO in this manner, you may want to use MsgWaitForMultipleObjects instead - this is what it is designed for.

I've made a few changes to the sample and have reattached it. Essentially I've moved from using messages in attempt to control the thread to using a shutdown event. In addition, the dialog *this* pointer is passed to the secondary thread which allows the 2nd thread to post an insert LV item msg to the dlg as well as wait on a shutdown event (this tells the 2nd thread to exit when the dialog is about to close). I've also added a CButton variable to the start button to disable the button so the user can't start multiple threads. Notice that I've removed the WM_USER messages (except for a new one that insert the new LV Item) and also have removed the m_bCanClose variable) as these are no longer needed.

Arjay

[MrViggy]

MrViggy, rather than using WFSO in this manner, you may want to use MsgWaitForMultipleObjects instead - this is what it is designed for.
Interesting function. I was not aware of that one!

Thanks!

Viggy

[Frunktz]

Ehm, the while(i < 10), it's used to simulate a long procedure in the thread, in the main app, that I developing...:)

Next, some post up this, there are a fix for this problem (thanks MrViggy)
PeekMessage()...

This method work great!
I have another question, about the use of PeekMessage...
With the loop, if I check the variabile m_hThread, that have a valid handle, until the thread works. When the thread has stopped, the variable have a invalid value:
My TRACE("%d\n", m_hThread), return -107482939 or similar value :)
Well, if I use that line

while (WaitForSingleObject(m_hThread, 0) != WAIT_OBJECT_0)

the loop, will not break, if I change the timeout, from 0 to 100, the loop exit normally.

Following the tip of MrViggy, I can check if m_hThread is valid:
if (m_hThread == NULL) break;
But, for set the variable to NULL, I must or I think to do this:

UINT MyThread(LPVOID pParam)
{
// some stuff...

m_hThread == NULL;
}

It's correct do this trick???

Ah, sorry for my bad bad English. I try to write my best...Sorry again

[MrViggy]

Hmm, that shouldn't happen, unless you previously closed that handle.

All handles to a thread must be closed before the system destroys them. Also, once the thread terminates, all those handles remain in a signaled state (which should trigger MsgWaitForMultipleObjects immediately).

Viggy

[Frunktz]

It's exact use "my" ideas? m_hThread = NULL, when the thread finish?

[MrViggy]

No. If 'm_hThread' is really a handle to a thread, then just setting it to NULL will result in a resource leak. Also, consider this situation:
1) Check 'm_hThread' is not NULL
2) Context switch! Thread now sets 'm_hThread' to NULL
3) ContextSwitch! GUI thread now calls 'WaitForSingleObject' on NULL thread handle.
This may be okay, however it's not really good coding.

Besides, you must close all handles you keep around. Otherwise, the thread will not completely be destroyed, until your application exits. How, exactly, is m_hThread initilaized?

Viggy

[Frunktz]

The m_Thread it's inizialized with

CWinThread* m_Thread;

m_Thread = AfxBeginThread(Foo, NULL);

[Arjay]

When the thread has stopped, the variable have a invalid value:By default AfxBeginThread() sets it's auto delete member to true. So it essentially performs a close handle (and deletes any allocated memory automatically). So when your thread exits, the m_Thread handle is closed and the pointer is deleted (although apparently not set to NULL). This is the reason for the invalid handle. You can change this behavior by setting m_bAutoDelete to false (see msdn). With it set to false you can get the thread exit code if you wish, but must remember to call CloseHandle and delete m_Thread to cleanup.



UINT MyThread(LPVOID pParam)
{
// some stuff...

m_hThread == NULL;
}

Because you are passing null in m_Thread = AfxBeginThread(Foo, NULL), you do not have access to the m_Thread variable from within the thread so you couldn't see this value to NULL (not that you would want to anyway).

Out of curiosity did you look at the modified sample code that I posted?

Arjay

[MrViggy]

Ahh, okay. So, as Arjay says, set the auto delete flag to false:
CWinThread *m_pMyThread = AfxBeginThread(MyThread, param, THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
m_pMyThread->m_bAutoDelete = FALSE;
m_pMyThread->ResumeThread();
Now, you should be able to query the thread handle, even after the thread function exits. Once you see that the thread is finished:
CloseHandle(m_pMyThread->m_hThread);
delete m_pMyThread;
And you're done!

Viggy

[Frunktz]

Well, good solution.
It's the most correct?
Set m_Thread = NULL, at the end of thread, isn't correct? If true, why?

[MrViggy]

You (possibly) don't have access to it (I can't tell where this var is declared). And, you have two threads accessing it, thus, you need to protect it (there's a chance that one thread reads the var while the other thread wants to change it).

Also, if m_hThread is indeed a handle, then there's also the possibility of a resource leak, if it's not closed. The method suggested by Arjay and myself is thread safe, and should work all the time.

Viggy