Thread questions (creating a wrapper class)

[MrDoomMaster]

I'm attempting to create a wrapper class for my own personal use. Something I can use for creating threads easily. I would also like for this class to implement safe threading, if possible.

I have developed this class based on articles I have read on the internet. There aren't many articles out there that have what I need (or at least I didn't find many), but I kind of put them all together. I was hoping that I could use the forums to help me fill in the gaps

I started off with the basics, I haven't implemented any code into most of the functions yet but I do have my basic class down... Here it is:


class Thread_Utils
{
private:
//--Variables-----------------------------------------------------------
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bool ThreadRunning ;
DWORD ThreadID ;
HANDLE ThreadHandle ;
//----------------------------------------------------------------------


//--Prototypes----------------------------------------------------------
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
static DWORD WINAPI ThreadProc(Thread_Utils* pThis);
//----------------------------------------------------------------------

public:
//--Prototypes----------------------------------------------------------
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/*--Constructors/Destructors--*/
Thread_Utils();
virtual ~Thread_Utils();

/*--Functions--*/
int StartThread(void);
int StopThread(void);
virtual DWORD ThreadCode(void);
//----------------------------------------------------------------------
};


At first, the static member function you see above was the most confusing, but I seem to have it down now.

Here is the definition of the static member function:


DWORD WINAPI Thread_Utils::ThreadProc(Thread_Utils* pThis)
{
return pThis->ThreadCode();
}


The way I understand this, is that when you call StartThread, it will create the thread using CreateThread() and send a this pointer as the parameter, and use the static member function as the entry point. The static function ThreadProc will immediately be called once StartThread is called. All ThreadProc does is call the virtual function ThreadCode for the current object.

My issue here is figuring out how to safely implement StopThread() so that it ends the thread safely. Basically, when I call StopThread, I want StopThread to wait until ThreadCode() is ready to safely exit, which means ThreadCode() will also need to detect when StopThread is called so that it may prepare to exit.

I have read MSDN on thread safety, but I seem to have a lot of trouble with their articles because they list about 8 different ways of implementing thread safety, while each of these methods has its own world of complexity.

I want my thread safety between StopThread() and ThreadCode() while having ThreadCode()'s implementation requirements to be as minimal as possible. Basically, I want to be able to code ThreadCode() without having to worry too much about adding safety features to it. I'm hoping my class can take care of most of that code without having to smash it into this function.

Here are some other function definitions I have created, as you can see, most are empty....



//===========================================================================================
/*
||
*/
Thread_Utils::Thread_Utils(void)
{
ThreadHandle = NULL;
ThreadID = 0;
}

//===========================================================================================
/*
||
*/
Thread_Utils::~Thread_Utils(void)
{
if(ThreadHandle)
StopThread();
}

//===========================================================================================
/*
||
*/
void Thread_Utils::StartThread(void)
{
ThreadHandle = CreateThread(NULL, 0, ThreadProc, this, 0, ThreadID);
}

//===========================================================================================
/*
||
*/
void Thread_Utils::StopThread(void)
{
// Need to terminate thread here

CloseHandle(ThreadHandle);
ThreadHandle = NULL;
}


If someone could provide me with examples, I would understand this better. This doesn't require a lot of code I hear, but I want to make sure I do it right, I need to understand the communication between threads so I don't make major mistakes in future implementations of threads in win32 programming.

[Andreas Masur]

[ Moved thread ]

[Andreas Masur]

Well...it is after midnight, so bare with me here...there are some points I would like to shortly mention.

The signature of your thread function (the static one) is not correct...take a look at the following FAQ (http://www.codeguru.com/forum/showthread.php?t=312452)...

Even with the correct signature, you do not have a way for the user of your class to provide parameters for the thread.

For information on how to end a thread safely you can use the following FAQ (http://www.codeguru.com/forum/showthread.php?t=305166)...

You have to think what happens, if the user does a copy or assignment with your class...you either need to prevent or handle it...

What do you mean with 'adding thread safety' in the context of your class?

[MrDoomMaster]

By thread safety I mean I want my class to wait for the current loop of the thread function to complete before it ends.

The StopThread() function and ThreadCode() function must communicate, by way of handles probably, to know when to end the thread.

A safe thread class would do this:

1. User calls StopThread()
2. StopThread() tells ThreadCode() to hurry and finish up so that StopThread() can close the thread, and return control.
3. ThreadCode() receives this notification, and completes its current iteration of the loop. When the loop iteration is complete, it tells StopThread() that it has completed the loop, and that StopThread() may now delete/terminate the thread.

an UNSAFE thread class would do this:

1. User calls StopThread()
2. StopThread() deletes/terminates ThreadCode(), regardless of the status of ThreadCode().

[Andreas Masur]

Okay...in this case, a safe way is described in the second FAQ I linked you to... :cool:

[MrDoomMaster]

Could you define what a signature is? Is this another way of saying header of the function?

I see the only difference in your example and in mine, is that you use LPVOID as your parameter instead of my Thread_Utils*.

Why would you use LPVOID? What extra functionality does this allow?

[MrDoomMaster]

Well, I was messing around and came up with some pretty cool features for my class. One thing I added that I've never seen anyone add in their thread classes is the ability to shut down the thread safely (by my definition above) without having to ever do a manual condition and return from the virtual worker function for the thread!

I haven't tested this yet, because I want opinions before I go off creating memory leaks or runtime errors due to my thread class!

Here is my CPP:


//===========================================================================================
// Function Definitions
//===========================================================================================
/*
|| // TODO: Provide descriptions for all functions
*/
DWORD WINAPI Thread_Utils::EntryPoint(LPVOID pThis)
{
while(true)
{
if(::WaitForSingleObject(((Thread_Utils*)pThis)->StopHandle, 0) == WAIT_OBJECT_0)
{
::SetEvent(((Thread_Utils*)pThis)->WaitHandle);
::ExitThread(0);
}

((Thread_Utils*)pThis)->ThreadCode();
}
}

//===========================================================================================
/*
||
*/
Thread_Utils::Thread_Utils(void)
{
ThreadHandle = NULL;
StopHandle = NULL;
WaitHandle = NULL;
ThreadID = 0;
}

//===========================================================================================
/*
||
*/
Thread_Utils::~Thread_Utils(void)
{
if(ThreadHandle)
StopThread();
}

//===========================================================================================
/*
||
*/
void Thread_Utils::StartThread(void)
{
StopHandle = ::CreateEvent(NULL, TRUE, FALSE, NULL);
WaitHandle = ::CreateEvent(NULL, TRUE, FALSE, NULL);
ThreadHandle = ::CreateThread(NULL, 0, EntryPoint, this, 0, &ThreadID);
}

//===========================================================================================
/*
||
*/
void Thread_Utils::StopThread(void)
{
::SetEvent(StopHandle);

::WaitForSingleObject(WaitHandle, INFINITE);

::CloseHandle(StopHandle);
::CloseHandle(WaitHandle);
::CloseHandle(ThreadHandle);

StopHandle = NULL;
WaitHandle = NULL;
ThreadHandle = NULL;
}

//===========================================================================================
/*
||
*/
void Thread_Utils::ThreadCode(void)
{

}


and my H :


class Thread_Utils
{
private:
//--Variables-----------------------------------------------------------
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bool ThreadRunning ;
DWORD ThreadID ;
HANDLE ThreadHandle ;
HANDLE StopHandle ;
HANDLE WaitHandle ;
//----------------------------------------------------------------------


//--Prototypes----------------------------------------------------------
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
static DWORD WINAPI EntryPoint(LPVOID pThis);
//----------------------------------------------------------------------

public:
//--Prototypes----------------------------------------------------------
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/*--Constructors/Destructors--*/
Thread_Utils();
virtual ~Thread_Utils();

/*--Functions--*/
void StartThread(void);
void StopThread(void);
virtual void ThreadCode(void);
//----------------------------------------------------------------------
};

[Andreas Masur]

Could you define what a signature is? Is this another way of saying header of the function?

With signature, I meant how the function is declared, in other words...

storage_class_qualifier return_datatype calling_convention function_name(function_args) const_qualifier exception_specification


I see the only difference in your example and in mine, is that you use LPVOID as your parameter instead of my Thread_Utils*.

Which was actually what I was talking about...

Why would you use LPVOID? What extra functionality does this allow?
Well...there is not extra functionality (except that using LPVOID would allow to pass more data than just a class pointer), the reason is simply that Windows expects this kind of function signature... :cool:

[ramesh chanda]

what is the disadvantage of constructor.

[Andreas Masur]

I haven't tested this yet, because I want opinions before I go off creating memory leaks or runtime errors due to my thread class!

Well...there are indeed some problems...which make the class not to work as expected.

The following code will not work.

while(true)
{
if(::WaitForSingleObject(((Thread_Utils*)pThis)->StopHandle, 0) == WAIT_OBJECT_0)
{
::SetEvent(((Thread_Utils*)pThis)->WaitHandle);
::ExitThread(0);
}

((Thread_Utils*)pThis)->ThreadCode();
}

It will simply block inside the function 'ThreadCode()'. Only if this function will return, the check for the event is performed. Since 'ThreadFunc()' is the thread function itself, it will not be terminated on its own. The check whether the thread actually needs to be terminated, needs to be inside the 'ThreadFunc()'.


The above problem will create another problem...'ThreadFunc()' is a user-supplied function...so, you need to find a way of getting the termination check inside this function...one way would be doing it like MFC which adds a call to the base class function (in your case it needs to be done inside the thread loop).

[Andreas Masur]

what is the disadvantage of constructor.
??? What do you mean with that?

[MrDoomMaster]

Why won't this work? You tell me HOW it won't work, but you don't tell me why really.

From what I've learned about C++, code will start from my thread's function, which I have renamed to "EntryPoint" instead of "ThreadFunc". It will then enter the while loop and check, the first time, to see if the event has been raised. after that, it will jump to the function call below, execute the code, and naturally it should return to the point it jumped, and then continue on until it hits the end of the while loop and start all over again.

The thread I create should be no different from the thread my code uses by default. Why would the code flow differently? I obviously have something setup wrong if it won't work as expected.

How can I adjust my function call inside of the function proc so that it will return successfuly (since you say it won't return) and allow the while loop to continue looping through it? MUST it return a value, instead of being void? If I do have it void, can I simply put a return; at the end of the function?

Thank you for your patience!

[MikeAThon]

Why won't this work? You tell me HOW it won't work, but you don't tell me why really.

Actually, Andreas did in fact tell you how that code wouldn't work. What he said was that after the WFSO releases, your ThreadCode() function will execute and you will never get a chance to go back to the WFSO until after ThreadCode() returns.

So, if ThreadCode() is a very short, iterative function that it guaranteed to finish one iteration quickly, then it will work as expected, in the sense that after ThreadCode() completes one of its iterations, the WFSO will test for termination and then exit.

But most ThreadCode() functions are not like that. Often, there's no guarantee that the code will complete (for example, if you're running a blocking socket in the thread). And many worker threads do not execute short iterative-style calculations, but rather execute long, non-repetitive tasks that can't neatly be categorized into iterative cycles.

If your goal is to encapsulate threads that iterate through a function that is, in fact, guaranteed to complete one iteration quickly, then your basic framework seems fine. The iteration function, incidentally, should be declared as virtual, and the encapsulation class should be abstract.

If your goal is a more fundamental encapsulation of any worker thread, then the class won't work for the reasons stated above.

Incidentally, one good article on class encapsulation of an iterative-type of worker thread is given by Paul DiLascia in his July 1998 C++ Q&A column from MSDN (then called MSJ). See http://www.microsoft.com/msj/0798/c0798.aspx . The article actually gives a non-threaded alternative to solving iterative-type workers without even spawning a thread; his threaded encapsualtion class is given as the answer to the second question. Code is included.

Regards,
Mike

[MrDoomMaster]

Hmm, that makes more sense!

What if I did use a blocking function though, such as accept() in winsock? or recv()?

I don't think a simple cancel button will make this function return immediately.

If I could find a way to make this function return, failed or not, then it would work fine, because I can control when it should break from the function whether a flag has been raised or not.

I liked his article, but I didn't really get anything out of it that could help me with this sort of issue, since you brought it to my mind.

Also I fear using asynchronous mode with winsock using two threads, because I wouldn't want the code overlapping itself somehow. Asynchronous means it has to leave the scope of the function to head to a procedure, which I don't think ANY of us want!

Thanks!

[MikeAThon]

Hmm, that makes more sense!

What if I did use a blocking function though, such as accept() in winsock? or recv()?

I don't think a simple cancel button will make this function return immediately.

That's correct, and there is no easy way around this other than asynchronous or event-based sockets.


If I could find a way to make this function return, failed or not, then it would work fine, because I can control when it should break from the function whether a flag has been raised or not.

I liked his article, but I didn't really get anything out of it that could help me with this sort of issue, since you brought it to my mind.
Since you're just starting out on this project, maybe it would be best to solve one problem at a time and learn about threads, before trying to solve all the world's problems ;)

Also I fear using asynchronous mode with winsock using two threads, because I wouldn't want the code overlapping itself somehow. Asynchronous means it has to leave the scope of the function to head to a procedure, which I don't think ANY of us want!

I don't understand what you mean by this. Asynchronous mode is terrific, as is event-based sockets which might be better in a worker thread which typically does not have a message pump. Of course it's a more complicated programming paradigm, but that's life.

Mike

[Arjay]

I see a couple of additional issues with the class:

1) Use of ::CreateThread(...) instead of _beginthreadex(...) function to start the thread. See "Programming Applications for Microsoft Windows" by Jeffery Richter. Chapter 6, "Thread Basics" extensively covers why _beginthreadex is preferred over ::CreateThread.

2) Why the while loop in Thread_Utils::EntryPoint(LPVOID pThis)? This may be already answered but usually a thread procedure is executed once. Users of your class may not expect their thread proc to be executed multiple times. In addition, you should pass the pThis pointer to the ThreadFunction so that the stop handle can be checked from within ThreadFunction.

3) Thread shutdown. If your thread is blocked, your current code will hang in the destructor. This may prevent your application from shutting down cleanly. Btw, what's the difference between WaitHandle and StopHandle? Do you need both events? It seems that you could use one event only and modify WFSO to wait on the thread handle instead of the event. Also, using the INFINITE wait here is dangerous. I would recommend signalling the thread to shutdown (by passing it the StopHandle event to check for within the thread), giving it a reasonable amount of time to shutdown, but then end the thread with TerminateThread as a last resort. NOTE: if you structure your thread function properly, the terminate thread code should rarely ever get called. If it gets called frequently something is wrong in the thread function code, but it's better to terminate the thread than to hang the main application.

4) Guard the start thread call. Currently, if a user calls StartThread multiple times (without calling StopThread in between), resources will be leaked (and you'll start multiple threads that you can't shutdown. You may choose to either close the current thread before firing a new one or to simply return.

5) No return codes. I've noticed you are using void functions. How does a user know if the methods in your class have succeeded or not?

6) Consider adding Pause() and Resume() methods.

Arjay

[MrDoomMaster]

Well to answer most of your concerns, I'm creating this class for PERSONAL use, as I had stated in my first post if I remember correctly. I don't plan to distribute this code, it is mine for my own personal benefit.

Thank you for your suggestions though, I will most definitely work on adding them.

[Arjay]

I appreciate you stating that your code is for personal use, but quite often what starts out as private sometimes ends up with someone else using or maintaining it.