Stopping a Thread
in [MFC]
|
From: Doug Harrison [MVP] on 16 Feb 2005 17:35 brogers5884 wrote: >In my application I have some list controls that load fairly large amounts of >data, a few thousand records, or the query to get the records just take a >really large amount of time. So I created threads to load the list controls >instead of locking up the front end while it tries to load and it works like >a champ. My problem being is while the list is loading if the user closes >that dialog by switching to another screen or trying to close the dialog the >program errors out and shuts down. So I put code in the destructor of the >list control that initiates that the thread: Sounds like the secondary thread doesn't like having the rug pulled out from under it. That is, you're destroying objects it's still using. You need to ask it to terminate and wait for it to do so before the list control is destroyed. A good place to do this might be in the dialog's OnDestroy message handler. Also, I don't know how your secondary thread is filling the list controls which belong to the primary thread, but it's questionable to manipulate them directly, especially while the user may be interacting with the control. There are two common ways to avoid problems with this: 1. Have the thread post information packets to a window in the main thread via a custom message (based on WM_APP or RegisterWindowMessage). The pattern looks something like this: Secondary Thread: X* p = new X; PostMessage(..., LPARAM(p)); Main Thread: LRESULT MyWnd::OnMyMessage(WPARAM, LPARAM lp) { X* p = (X*) lp; ... use p delete p; ... } 2. Set up a timer in the main thread and examine the relevant data structure at regular intervals. For a job that collects data non-stop until it terminates, I found this worked great for a virtual listview control, where adding items to the listview means simply calling the SetItemCount function. You will need to synchronize access to the data structure, though. >m_pThread->PostThreadMessage(WM_USER_STOPTHREAD,0,0); >while(WaitForSingleObject(m_pThread->m_hThread, 1000) != WAIT_FAILED); > >Now I don't get immediate severe errors but the program hangs indefinitely >on the WaitForSingleObject command. Any ideas on how I can get that thread >to stop it's execution so the window can close gracefully and not lock up the >program? Thank you. Sounds like either the secondary thread isn't running a message loop, so it never notices the request, or the return value you're expecting never comes. All things being equal, you should replace your loop with: WaitForSingleObject(m_pThread->m_hThread, INFINITE); Checking the return value might be useful for debugging, but not for much else here. It sounds like you're making a number of common mistakes when using CWinThread. Please see this message on how to avoid them: http://groups-beta.google.com/group/microsoft.public.vc.mfc/msg/430fb37711fbf662 In particular, if you're allowing the CWinThread to auto-delete, your WFSO is undefined for the reason given in that message, plus your m_pThread pointer may have been invalidated while you're still using it. And if the thread isn't auto-deleting, the handle is never being closed, so your WFSO will either return WAIT_OBJECT_0 or WAIT_TIMEOUT forever. -- Doug Harrison Microsoft MVP - Visual C++
From: brogers5884 on 16 Feb 2005 18:09 Excellent, thank you very much. "Doug Harrison [MVP]" wrote: > brogers5884 wrote: > > >In my application I have some list controls that load fairly large amounts of > >data, a few thousand records, or the query to get the records just take a > >really large amount of time. So I created threads to load the list controls > >instead of locking up the front end while it tries to load and it works like > >a champ. My problem being is while the list is loading if the user closes > >that dialog by switching to another screen or trying to close the dialog the > >program errors out and shuts down. So I put code in the destructor of the > >list control that initiates that the thread: > > Sounds like the secondary thread doesn't like having the rug pulled out from > under it. That is, you're destroying objects it's still using. You need to > ask it to terminate and wait for it to do so before the list control is > destroyed. A good place to do this might be in the dialog's OnDestroy > message handler. > > Also, I don't know how your secondary thread is filling the list controls > which belong to the primary thread, but it's questionable to manipulate them > directly, especially while the user may be interacting with the control. > There are two common ways to avoid problems with this: > > 1. Have the thread post information packets to a window in the main thread > via a custom message (based on WM_APP or RegisterWindowMessage). The pattern > looks something like this: > > Secondary Thread: > > X* p = new X; > PostMessage(..., LPARAM(p)); > > Main Thread: > > LRESULT MyWnd::OnMyMessage(WPARAM, LPARAM lp) > { > X* p = (X*) lp; > ... use p > delete p; > ... > } > > 2. Set up a timer in the main thread and examine the relevant data structure > at regular intervals. For a job that collects data non-stop until it > terminates, I found this worked great for a virtual listview control, where > adding items to the listview means simply calling the SetItemCount function. > You will need to synchronize access to the data structure, though. > > >m_pThread->PostThreadMessage(WM_USER_STOPTHREAD,0,0); > >while(WaitForSingleObject(m_pThread->m_hThread, 1000) != WAIT_FAILED); > > > >Now I don't get immediate severe errors but the program hangs indefinitely > >on the WaitForSingleObject command. Any ideas on how I can get that thread > >to stop it's execution so the window can close gracefully and not lock up the > >program? Thank you. > > Sounds like either the secondary thread isn't running a message loop, so it > never notices the request, or the return value you're expecting never comes. > All things being equal, you should replace your loop with: > > WaitForSingleObject(m_pThread->m_hThread, INFINITE); > > Checking the return value might be useful for debugging, but not for much > else here. > > It sounds like you're making a number of common mistakes when using > CWinThread. Please see this message on how to avoid them: > > http://groups-beta.google.com/group/microsoft.public.vc.mfc/msg/430fb37711fbf662 > > In particular, if you're allowing the CWinThread to auto-delete, your WFSO > is undefined for the reason given in that message, plus your m_pThread > pointer may have been invalidated while you're still using it. And if the > thread isn't auto-deleting, the handle is never being closed, so your WFSO > will either return WAIT_OBJECT_0 or WAIT_TIMEOUT forever. > > -- > Doug Harrison > Microsoft MVP - Visual C++ >
From: Joseph M. Newcomer on 16 Feb 2005 22:33
You are confused about what it means to have a thread exit. A thread is running asynchronously. If you do something that causes it to terminate, it will terminate sometime in the indefinite future, and that is all you can know. There is no clean way to stop a thread immediately. It is your responsibility, as the coder of the thread, to ensure that the delay is not undue. Also, it sounds suspiciously like the thread is trying to manipulate the tree control, which represents a fatal design error you will have to fix, if that is what you are doing. Key here: you ask the thread to stop. At some future point, it stops. You cannot allow anything the thread depends upon to go away until the thread has stopped. That's all there is to it. Now, you say the thread is fetching records. If it is blocked waiting for something to produce each record, then it cannot possibly stop until it unblocks from that situation. Then, it has to check each time through its loop if it should stop. By the time you get to the destructor, the dialog and all its child controls are gone. This is why the destructor is about the most useless possible place to shut the thread down, because it is far too late at that point. The correct approach goes something like this: when the request comes in to close the window, you look to see if a thread is active. If the thread is not active, you just call the normal close handler, such as calling CDialog::OnOK or CFormView::OnClose. But if the thread is active, you set a flag indicating shutdown-in-progress and simply return without doing anything. You probably want to update all the controls to disable them, and if it is a CFormView, you set all the menu handlers to disable the menu items so nothing else happens. The thread is checking this shutdown-request flag (or a reflection of it), and when it sees it, it stops what it is doing, cleans itself up, and posts a message back saying that it is now finished. The message handler for the GUI receives this message, and if it is in a shutdown situation, it now callls CDialog::OnOK or CFormView::OnClose to complete the closing of the app or window. No wait is involved. But any assumption that you can "just stop" a thread is unfounded; it is intrinsic in the nature of threads that this is impossible. And a thread must not manipulate any window belonging to the GUI thread. Period. You will get into serious trouble eventually. If it appears to be working, that is an illusion; in fact, your application hanging is a place where the illusion broke down. joe On Wed, 16 Feb 2005 14:09:04 -0800, "brogers5884" <brogers5884(a)discussions.microsoft.com> wrote: >I don't want to wait for it at all, I want it to exit immediately. In the >thread it just gets a recordset and then loops through the recordset and >loads each record into a custom drawn list control. If you try to close that >dialog while the thread is in it's loop loading the list control it blows up >in glorious fashion each and every time. I'm just trying to find some way to >tell that thread to stop doing anything and get out. Thanks for all of the >responses. > >"Joseph M. Newcomer" wrote: > >> This works only if the thread returning to the message pump. And the WaitForSingleObject >> is almost certainly fatal. The fact that it times out is even more suspicious. In fact, >> the piece of code is remarkably silly, in that it simply loops; INFINITE would have been a >> more effective timeout. >> >> But why do you have to wait for it at all? >> >> And putting it in the destructor is probably the wrong place to put it. >> >> You should shut the thread down in the OnClose handler. The destructor is far too late. >> >> Read my essay on worker threads on my MVP Tips site. >> joe >> >> On Wed, 16 Feb 2005 12:53:01 -0800, "brogers5884" <brogers5884(a)discussions.microsoft.com> >> wrote: >> >> >In my application I have some list controls that load fairly large amounts of >> >data, a few thousand records, or the query to get the records just take a >> >really large amount of time. So I created threads to load the list controls >> >instead of locking up the front end while it tries to load and it works like >> >a champ. My problem being is while the list is loading if the user closes >> >that dialog by switching to another screen or trying to close the dialog the >> >program errors out and shuts down. So I put code in the destructor of the >> >list control that initiates that the thread: >> > >> >m_pThread->PostThreadMessage(WM_USER_STOPTHREAD,0,0); >> >while(WaitForSingleObject(m_pThread->m_hThread, 1000) != WAIT_FAILED); >> > >> >Now I don't get immediate severe errors but the program hangs indefinitely >> >on the WaitForSingleObject command. Any ideas on how I can get that thread >> >to stop it's execution so the window can close gracefully and not lock up the >> >program? Thank you. >> >> Joseph M. Newcomer [MVP] >> email: newcomer(a)flounder.com >> Web: http://www.flounder.com >> MVP Tips: http://www.flounder.com/mvp_tips.htm >> Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm |