[RESOLVED] Why doesn't this method of closing a worker thread work?
CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Page 1 of 2 12 LastLast
Results 1 to 15 of 22

Thread: [RESOLVED] Why doesn't this method of closing a worker thread work?

  1. #1
    Join Date
    Apr 2017
    Posts
    12

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

    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

    Code:
    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:

    Code:
    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:

    Code:
    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?

  2. #2
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    5,451

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

    In startThread don't you need a loop that checks flag rather than just 1 test at the beginning?

    See http://flounder.com/workerthreads.htm
    All advice is offered in good faith only. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/

    C, C++ Compiler: Microsoft VS2017.2

  3. #3
    Join Date
    Apr 2017
    Posts
    12

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

    The StartThread function is called periodically by SetTimer. That is why I did not put it in a loop.

  4. #4
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    5,451

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

    Quote Originally Posted by VBCherry View Post
    The StartThread function is called periodically by SetTimer. That is why I did not put it in a loop.
    with the same param pointer? - every time OnTimer() is called, param is set to point to new allocated memory.

    PS Where are you freeing the allocated memory?
    All advice is offered in good faith only. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/

    C, C++ Compiler: Microsoft VS2017.2

  5. #5
    VictorN's Avatar
    VictorN is offline Super Moderator Power Poster
    Join Date
    Jan 2003
    Location
    Wallisellen (ZH), Switzerland
    Posts
    18,907

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

    Quote Originally Posted by VBCherry View Post
    The StartThread function is called periodically by SetTimer. That is why I did not put it in a loop.
    No, it is called only once in each thread you start!
    And note that you start a new thread each time your OnTimer handler is called...
    Note also that you lose access to every new created THREADSTRUCT instance right after your OnTimer returns, so you won't be able to change any value of those THREADSTRUCT instances....
    Victor Nijegorodov

  6. #6
    Join Date
    Apr 2017
    Posts
    12

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

    @VictorN @2kaud

    Thank you for pointing out the mistake I was making. I changed the code so that I declared the THREADSTRUCT instance in the header file, and initialised it and it's members in the OnStartButton function. This was I know that I am passing the same structure every time. I delete the _param pointer in the function associated with the stop button.
    I also realised that SetTimer has a KillTimer function to stop it, which I thought would be a easier way of stopping creation of new threads. But this just hangs my GUI for a bit and then resumes whatever the thread was doing. I'm not sure why my KillTimer isn't working.

    But thank you for pointing out this mistake!!!

  7. #7
    Join Date
    Apr 2017
    Posts
    12

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

    @VictorN

    I was thinking about why the thread wasn't end when I killed the timer, is it possible that the thread doesn't end after the thread function goes out of scope each time?

  8. #8
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    5,451

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

    I also realised that SetTimer has a KillTimer function to stop it, which I thought would be a easier way of stopping creation of new threads
    Yes. But this has nothing to do with terminating existing threads.

    is it possible that the thread doesn't end after the thread function goes out of scope each time?
    A thread ends when the code for the thread finishes running - or the thread is abnormally terminated (avoid). The scope of the thread function in this context is irrelevant.
    All advice is offered in good faith only. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/

    C, C++ Compiler: Microsoft VS2017.2

  9. #9
    Join Date
    Apr 2017
    Posts
    12

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

    Quote Originally Posted by 2kaud View Post
    Yes. But this has nothing to do with terminating existing threads.
    But if it is my timer that creates new threads after the old one have ended (return 0), won't killing that timer stop further threads from being created?
    I am terribly sorry if I am making this unnecessarily complicated. It's just that I want what ever the thread does to stop when I press stop, and I assumed killing the timer would do this. But this just causes my GUI to hang.
    If it helps, the application is supposed to read from a serial port every 50 milliseconds and display the data on the GUI when the user presses start. This is why I put the read function in the thread (so that the GUI can still be interacted with) and the thread in the timer. I open the port and initialise the' _param' parameter outside the thread. On stop, I kill the timer and close the serial port handle. But this causes the aforementioned hang.

  10. #10
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    5,451

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

    But if it is my timer that creates new threads after the old one have ended (return 0), won't killing that timer stop further threads from being created?
    How do you know when the old thread has ended? - there's nothing in the posted code that determines this. Killing the timer will stop new threads being created but this doesn't terminate existing threads. Possibly the 'hang' is caused because on stop you close the serial port handle from which existing threads are still trying to read?
    All advice is offered in good faith only. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/

    C, C++ Compiler: Microsoft VS2017.2

  11. #11
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    5,451

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

    to read from a serial port every 50 milliseconds and display the data on the GUI when the user presses start
    But why start a new thread for every read? Why not have a timer within the thread function itself? Why not start the port thread once when the main program starts - but suspended. When start button clicked, resume the thread and read port every 50 milliseconds. When stop button clicked suspend the thread. When the main program finishes, the thread will automatically be terminated.
    All advice is offered in good faith only. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/

    C, C++ Compiler: Microsoft VS2017.2

  12. #12
    Join Date
    Apr 2017
    Posts
    12

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

    Quote Originally Posted by 2kaud View Post
    But why start a new thread for every read? When start button clicked, resume the thread and read port every 50 milliseconds.
    Is there a way to automatically resume a suspended thread after a certain amount of time? Also does the suspension and resumption happen from within the thread? This could potentially make my code smaller and more efficient!

    EDIT: I found the related notes in the website you linked. Can those functions be called from both outside and inside the thread? And do I use Sleep() between them?
    Also AfxBeginThread returns a CWinThread pointer, is this an identification for the thread? So can I store this in a pointer and call SuspendThread(CWinThread* pPointer)?
    Last edited by VBCherry; June 10th, 2017 at 05:44 AM.

  13. #13
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    5,451

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

    Resuming a suspended thread has to be done outside of the thread - as its suspended! However you can use a timer within a thread to give the approx 50ms port read requirement. As mentioned in post #11, why not create the thread suspended once at the start of the program (and save its return value as this is a pointer to the CWinThread object eg pthrd) and when the user clicks the start button, resume the thread (pthrd->ResumeThread()) and when clicks the stop button suspend the thread (pthrd->SuspendThread()) ?
    All advice is offered in good faith only. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/

    C, C++ Compiler: Microsoft VS2017.2

  14. #14
    Join Date
    Apr 2017
    Posts
    12

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

    Quote Originally Posted by 2kaud View Post
    Resuming a suspended thread has to be done outside of the thread - as its suspended! However you can use a timer within a thread to give the approx 50ms port read requirement. As mentioned in post #11, why not create the thread suspended once at the start of the program (and save its return value as this is a pointer to the CWinThread object eg pthrd) and when the user clicks the start button, resume the thread (pthrd->ResumeThread()) and when clicks the stop button suspend the thread (pthrd->SuspendThread()) ?
    This is a brilliant idea! I do have one hypothetical to run by you. (I am terribly sorry for taking your time, but it is not often that I have someone experience guiding me).
    Can use the timer to callback a function that resumes the thread every 50ms, and have the thread suspend itself each time? I don't know how to use SetTimer inside a thread as I have to define a callback function, and I don't know how to pass this into the thread.

  15. #15
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    5,451

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

    The easiest way is probably to sleep the thread for 50ms. Consider for the thread function
    Code:
    while (true) {
       //read port
       //process port data
       Sleep(50);
    }
    All advice is offered in good faith only. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/

    C, C++ Compiler: Microsoft VS2017.2

Page 1 of 2 12 LastLast

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  


Windows Mobile Development Center


Click Here to Expand Forum to Full Width

This a Codeguru.com survey!


On-Demand Webinars (sponsored)