Re: Bad practice with an MFC thread? How can I improve it?
On Thu, 17 Apr 2008 14:56:04 +0100, "Moschops"
<moschops@fearofstrangers.co.uk> wrote:
I've had to alter some code I was handed. A live camera was constantly
filling a ring buffer with images, and on a given signal (which I've put in
as a button push for the moment) it stopped, leaving the last X images in
the buffer. The requirement came in such that it would run for X/2 more
frames after the button was pushed (i.e. when you push the button, the saved
video sequence is centred at the time of pushing the button).
Initially, I added a frame counter after the button was pushed, but found
that it caused the processor to spend all the time on counting and no more
time on updating the frames, so in essence it hung. Ideal time for threads,
I decided.
I understood the first paragraph, but I lost the picture (pun intended) on
the second one.
So now pushing the button calls an AfxBeginThread function, like this:
// Spawn thread to decide when to stop capturing
CWinThread* pStopCaptureThread;
pStopCaptureThread = AfxBeginThread (
fnStopCaptureThread,this,THREAD_PRIORITY_NORMAL,0,0,NULL);
That's an unsafe usage of AfxBeginThread. For guidance on using it safely,
see:
http://members.cox.net/doug_web/threads.htm
Now, I understand that the fnStopCaptureThread function cannot be part of
the class within which this is all happening, so it's a global function.
However, I still need access to various class variables to do my
calculations, so that global function simply calls another function within
the original class object; the fnStopCaptureThread knows what object called
it because it is passed a 'this'.
You can use a private static member function for the thread controlling
function instead of a global function. Helps with encapsulation.
Here's the body of that global function:
UINT fnStopCaptureThread(LPVOID lParam)
{
// Run the stopcapture code
Sequencer_class* dlg2 = (Sequencer_class*)lParam; // This kind of casting
is not the C++ way. Needs fixing. Moschops.
dlg2->StopCapture();
return 0;
}
StopCapture is the function, within the class, that handles counting frames
and working out when to stop. This all seems happy now, and it behaves as
expected thus far.
What happens if someone closes your dialog or continues to interact with it
while StopCapture is running? That's always a concern when you move from a
modal operation to an asynchronous one.
Looking at my code, I am aware that I've now got a thread playing with my
class object, and the code that spawned that thread is also still playing
with the exact same object, albeit never calling the StopCapture function.
I'm got this feeling that it's a bad idea to have in essence two threads
accessing the same object - presumably I won't be able to predict when
they'll do things relative to each other and could end up with problems.
This code section is quite simple and the variables StopCapture() is playing
with aren't touched by anything else until the thread is killed, but
nonetheless is seems like bad practice. My MFC experience is quite limited
(I'm usually an embedded coder) so I'm not sure if I'm seeing things, or if
this is a bad idea. Can anyone help me out? Is this bad practice, and if so,
what's a better way to do it?
You're right to be concerned. If I understand correctly, you originally had
the dialog object calling StopCapture, which blocked the whole program
while it was running. Is that really such a bad thing? Lots of programs
display the hourglass and become unresponsive while they're doing work that
must be completed before they can proceed. I'd worry about it only if it's
a lengthy period of unresponsiveness, because when you start offloading the
work onto a secondary thread and allow the primary thread to continue
processing messages, you create some complications. The primary thread,
which previously was blocked while the work was being done, must now take
the concurrent execution of the secondary thread into consideration and do
nothing that's going to mess it up. This can mean synchronizing access to
shared data structures, not allowing the dialog to exit while the secondary
thread is still using it, and so forth. Read the article I linked to
earlier for more on the CWinThread life cycle, and what you must do to use
it robustly.
Also, realize that when your secondary thread calls functions in your
dialog class, you can very easily end up using inter-thread SendMessage, as
most window functions involve SendMessage under the hood. This takes some
care to avoid deadlock; see my message from yesterday in the thread "when
my window talks to my thread" for more on this.
--
Doug Harrison
Visual C++ MVP