Archived

This topic is now archived and is closed to further replies.

kuphryn

Major Multithread Problem :: MFC

Recommended Posts

Hi. I just found a major multithread bug a program I working on. First, I will describe the process that is causes the major multithread problem. ----- - user opens a text file (any size) w/ text only - user clicks an option in the menu - view has a executes its message handler for the menu item - view calls a function in doc - doc begins a new thread (thread includes CSinglelock + CriticalSection, data from doc) - thread resumes and begins processing the data - when done, thread *PostMessage* mainframe that is done ::: -> AfxGetMainWnd->PostMessage(WM_USER_THREAD, 0, 0); - main frame *SendMessage* to view ::: -> pView->SendMessage(WM_USER_THREAD, 0, 0) - view calls a function in doc to close thread - doc class this function ----- //// // This function is called to close a thread // m_bThread = true; // meaning thread is still active if (m_pThread && m_hThread) // m_hThread is a HANDLE to the thread { ::WaitForSingleObject(m_hThread, INFINITE); delete m_pThread; m_pThread = NULL; m_hThread = NULL; m_bThread = false; // meaning thread has been closed SetModifiedFlag(TRUE); } UpdateAllViews(NULL); ----- Okay. Everything works good *if the use does not click anything* during the process. In other words, if the user were to click the menu or anything inside the window, the program would malfunction. Notice the bool "m_bThread." I use it as an indicator about the status of the thread. if m_bThread is true, meaning that the thread is active, the program disables many options in the menu. These options will remain disabled unless m_nThread is false. For some reason, that variable remains *true* if the user starts clicks anyone in the menu or view processing. That means something is keeping the program to ever setting m_Thread to false. On the other hand, if the user does not click anything until after the process is over, everything works as planned. I have no idea what is going on in the background. Is there a design flaw anywhere? Thanks, Kuphryn

Share this post


Link to post
Share on other sites
Couple of things.

1. The thread should clean up itself. Why does the document need to tell the thread to exit? If you want to "tell" a thread to exit you should have an "Exit" event that it waits on. Besides you can''t delete a thread like you''re doing. If you''re exiting the thread from the thread itself, call ExitThread. Killing the thread outside of the thread you use TerminateThread.

TerminateThread does NOT free resources allocated inside your thread. It just removes it from the OS''s list of threads. Read about it.

2. Need more detail than what you''ve given. Is the thread created every time the menu item is selected and closed after posting it''s message to the mainframe?

A better idea is to have events that represent your threads current state and for telling the thread to do stuff.

Events controlling thread:
START - process a chunk of data
EXIT - self-explanatory

Events showing thread state:
BUSY - set for working, not set for ready for more data

Make your events in an array so you can ::WaitForMultipleObjects on the control events, determine which one it is, and do the right thing.

Periodically call ::WaitForSingleObject(hBusyEvent, 0) on the BUSY event during a WM_TIMER tick in your View or Mainframe. This way you''re only checking the state of the event and not waiting around for the event to be set.

Create the timer when the menu button is pressed and destroy the timer when the BUSY event isn''t set anymore.

With this methodology you keep the thread around until the application is closed rather then recreating the same thread each time a data unit needs processing.

Hope you understand.

Good luck.

Share this post


Link to post
Share on other sites
Thanks.

The program creates the worker thread on the heap and deletes it after each event (i.e each time the user clicks that option).

I am not familiar with ::ExitThread and have not implemented it at the end of the thread function, assuming that is where I should add it. I did read about ::TerminateThread in Prosise''s book where he recommended not to use that Afx function.

I still have no idea what is causing the thread to not close *or* main not calling the function to delete the pointer to the thread and reassign m_bThread to false (meaning thread is closed).

Kuphryn

Share this post


Link to post
Share on other sites
Hi.

I believe I have figured out the problem. Here is a scenario. Let say you have a program with splitter window. There are two views and both are CEditView. Let say the top view calls a worker thread. While the worker thread is active, you focus the lower view and begin typing. What is the chance that the worker thread will never get deleted?

I just simulated the scenario above using a very simple program with a progress bar. If I focus on another window wile the progress bar is active, the program will not delete the progress bar, period.

I can email or upload the source code to that simple simulation anyone wants to see it.

Thanks,
Kuphryn

Share this post


Link to post
Share on other sites
So many red flags, so little time. I''m a wee bit tired right now so I apologize if this isn''t as helpful as it could be.

1) Never use exitThread, terminateThread, etc. Well-behaved worker threads simply return from the worker thread function, which releases the thread resources. Usually the worker thread has a bool or a full-fledged Event that it checks in a loop; if the UI sets this signal, the worker thread should gracefully return.

2) Why do you have an m_pThread and an m_hThread? If you m_pThread is a CWinThread*, it already has an m_hThread member.

3) The worker thread should be oblivious to messages happening in your UI thread. If the thread never exits, it means you never told it to.

4) Can we see exactly how you''re starting this thread?

Share this post


Link to post
Share on other sites
Okay. Thanks.

Answer: m_hThread is used for waiting: "::OnWaitSingleObject" or something like that. I need to to wait to make sure the thread has been closed.

I *believe* I might now the problem. For some reason, MFC does not like the following process:

-----
- worker thread finishes (end of function)
- worker thread PostMessage main window an update
- main SendMessage view (basically redirecting the same message
- view class a function in doc to close thread (delete pointer)
- function in doc delete the pointer to CWinThread
-----

MFC seems to like this design:

-----
- worker thread finishes (end of function)
- worker thread PostMessage main window an update
- main calls a function in doc to close thread (delete pointer)
- function in doc delete the pointer to CWinThread
-----

Notice in the second algorithm, main calls the function in doc to delete the thread *directly* instead of sending view a message.

Could that be the problem? The simulation program I mentioned seems to function much better under the second algorithm.

Kuphryn

Share this post


Link to post
Share on other sites
CWinThread already has an m_hThread. You should do this:
::WaitForSingleObject (m_pThread->m_hThread, INFINITE);

CWinThread also auto-deletes itself if you have the m_bAutoDelete set (which it is by default). This means that you should not be deleting it at all if you set this, or you should be clearing m_bAutoDelete if you want to delete it yourself. Sounds like this is the root of your problem.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Okay.

What do you mean by "clearing m_bAutoDelete?" I sent it to FALSE before resuming the thread.

Kuphryn

Share this post


Link to post
Share on other sites
Okay. I did set it to FALSE before resuming the worker thread.

I did some code manipulation, mainly moving some code from view to main. I also passed into the worker thread fucking a point to main window. Before, i used AfxGetMainWnd() to post a message to main frame. Now, I use the pointer to the main window: pWnd->PostMessage().

Secondly, main calls the function in doc to close the worker thread (set m_bThread to false). I do not know if that was the problem in the first place. I will keep debugging it. Nonetheless, the program process data quicker now. Maybe too many calls to PostMessage and SendMessage can overflowed the message queue.

Kuphryn

Share this post


Link to post
Share on other sites