Why doesn't this method of closing a worker thread work?

Question

0.00/5 (No votes)

See more:

I have an MFC application whose GUI has a start and a stop button. The start button calls a function which I want to run in the background, periodically. My solution was to use the SetTimer function to call the OnTimer function at regular intervals. This function then creates a thread using AfxBeginThread and then populates the structure that I pass to the thread as a parameter. All this works just fine.

What I would like to do now is the make the stop button end the thread. I have read about TerminateThread but it seems too risky. Since I want to close the worker thread from another thread, I know I can't use AfxEndThread, but I wanted to know why the following solution doesn't work.

What I have tried:

I created an int variable called count and set it to 0 whenever start is pressed, and set it to 1 whenever stop is pressed.
The structure I am passing into AfxBeginThread looks like this

typedef struct THREADSTRUCT
            {
            MyGUIDlg*    _this;
	       int          flag;
            //you can add here other parameters you might be interested on
           } THREADSTRUCT;

When start is pressed the SetTimer function is called, whose callback function is defined like this:

void MyGUIDlg::OnTimer(UINT_PTR nIDEvent)
{
	THREADSTRUCT* _param = new THREADSTRUCT;
	_param->_this=this;
	_param->flag = count;
	AfxBeginThread(StartThread, _param);	
	CDialog::OnTimer(nIDEvent);
}

In my StartThread function I have added the following:

UINT MyGUIDlg::StartThread(LPVOID param)
{
    THREADSTRUCT *ts = (THREADSTRUCT*)param;
    if((ts->flag)==1)
	{
		//Use AfxEndThread to close this thread
	}

	return 0;
}

But this doesn't seem to work, any thoughts why?

Posted 9-Jun-17 20:48pm

TheLostJedi

Updated 9-Jun-17 21:53pm

v3

Add a Solution

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Jochen Arndt · Accepted Answer · 2017-06-09T21:53:00

Solution 1

A worker thread usually contains a loop where it checks for a stop condition:

UINT MyGUIDlg::StartThread(LPVOID param)
{
    THREADSTRUCT *ts = (THREADSTRUCT*)param;
    while (!ts->flag)
    {
        // Perform operations here
        // May also break the loop for self termination
    }
    // Thread is closed when it returns or when
    //  AfxEndThread() is called.
    // When m_bAutoDelete is TRUE, the thread object is deleted.
    return 0;
}

The above polls a shared variable variable to check if the thread should terminate. A better solution is using some kind of signals (events).

But in your example there is no loop and the thread is always closed immediately.

I suggest to read about worker threads. An old but very good article is Using Worker Threads[^]. See also Multithreading with C++ and MFC[^] in the MSDN.

Posted 9-Jun-17 21:53pm

Jochen Arndt

Comments

TheLostJedi 10-Jun-17 4:58am

Thank you! The StartThread function itself is called periodically by SetTimer, that is why I didn't put a loop. But I am going to look into using signals to close the thread.

Jochen Arndt 10-Jun-17 5:06am

Starting a thread from a timer is a bad approach (I had in mind to mention that in my solution but forgot it). It will start a new thread periodically so that you might have multiple threads running. Then you must use some kind of thread pool to track the state of each thread so that you can stop them individually (that is: have a THREADSTRUCT for each thread). If you only want to start a single thread there is no need for a timer and you can check the state of running threads before starting a new one (which requires storing the thread object pointer).

I don't know about your requirements (what you are doing in the thread) but you might rethink your design. The provided links should help with that.

TheLostJedi 10-Jun-17 5:11am

But doesn't the "return 0" statement end the thread each time? I need to do the function carried out by the thread periodically, that is why I used SetTimer. I thought about using the SetTimer function from within a single thread, but I wasn't sure as to how I would pass it the OnTimer function?
I am still working on the design, so I'm all ears for useful changes.

Jochen Arndt 10-Jun-17 5:52am

It ends the thread. If it is a short running thread then you usually don't need a stop condition (besides when terminating the application). If it is a long running thread you might get multiple threads from the timer.

If the thread should do some work periodically, it might be better to do the timing within the thread using WaitForSingleObject or WaitForMultipleObjects with an appropriate time out value. These wait functions can be also used to act upon signals (events) like thread termination and perform work before the time out occurs.