Jump to content
  • Advertisement
Sign in to follow this  
hkBattousai

[C++/Win32] My thread class is not working

This topic is 3122 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I wrote down the class below for general threading needs in Win32 programs. Win32 API is written for C, and sometimes can have inconsistancies with CPP. For instance, the third parameter of the Win32 API function CreateThread() must be a pointer to a function in the form of "DWORD WINAPI FunctionName(LPVOID lpParameter);". However, if you decide to use this API function in a CPP class you encounter a problem. When I wrote down my threading class code for the first time, couldn't compile it. The compiler gave an irrelevent error message which wasn't describing the real problem in the code. After googling and comparing with sample codes I understood what the problem was. I learned that all C++ member functions had a hidden first parameter, which is nothing but a pointer to the class they are in. I didn't know how to deal with this hidden parameter, actually I had no idea, I was stuck. So I used sample codes I found on the net to write my own code. Every sample code has these common similarities: 1) Class method which will be passed to CreateThread() API function (in my code, it is the ThreadProc() method) is declared to be static. 2) The parameter passed to ThreadProc() is the pointer to the class itself (i.e.; the "this" pointer); 3) "this" pointer is passed to the fourth parameter of CreateThread(). My code is working if you define only one instance of the class. But it gives a runtime error if you define a second instance. I think, it has something to do with the static ThreadProc() method. Can anyone plase answer my questions about all these confusion: 1) Why do we have to define ThreadProc() static? What does it change? 2) I didn't understand the relation between every method having a hidden first parameter "this", and the fact that we passed "this" (via the fourth parameter of CreateThread()) as the first parameter. It already had a hidden "this", and we passed a second one. It made two "this"es, didn't it? Why did we need to pass a second "this"? What is the logic behind this? 3) How should I modify my code, so that I can declare multiple instances of my class without any runtime errors? (IDE : Visual Studio 2005 SP 1) GenericThread.cpp
#pragma once
#include <Windows.h>

class GenericThread
{
	public:
		GenericThread();
		~GenericThread();
		
		// Create and run the thread
		HANDLE StartThread(int nPriority = 0, bool create_suspended = false, SIZE_T stack_size = 0, bool reserve_stack = false);
		
		// This member function will run when you call StartThread()
		static DWORD WINAPI ThreadProc(void * lpParameter);
		
		// The run() method which will be called for threading
		virtual void run();
		
		// -15 : lowest; +15 : highest
		void SetThreadPriority(int nPriority);
		
		// Get currently assigned priority level
		int GetThreadPriority();
		
		// Get thread status
		bool IsThreadRunning();
		
		// Pause execution
		void SuspendThread();
		
		// Resume execution
		void ResumeThread();
		
		// Terminate execution
		void TerminateThread();
		
		// Return thread handle
		HANDLE GetThreadHandle();
		
	private:
		// Handle to the thread
		HANDLE m_hThread;
		
		// The thread id assigned by Windows, there is no use for now
		LPDWORD m_lpdwThreadID;
		
		// Thread status
		bool m_bRunning;
};
GenericThread.h
#include "GenericThread.h"

GenericThread::GenericThread()
{
	m_bExiting = false;
	m_bRunning = false;
}

GenericThread::~GenericThread()
{
	if (m_bRunning) CloseHandle(m_hThread);
}

HANDLE GenericThread::StartThread(int nPriority /*= 0*/, bool create_suspended /*= false*/, SIZE_T stack_size /*= 0*/, bool reserve_stack /*= false*/)
{
	DWORD dwCreationFlags = 0;
	if (create_suspended) dwCreationFlags |= CREATE_SUSPENDED;
	if (reserve_stack) dwCreationFlags |= STACK_SIZE_PARAM_IS_A_RESERVATION;
	m_hThread = CreateThread(NULL, stack_size, (LPTHREAD_START_ROUTINE) ThreadProc, (void *) this, dwCreationFlags, m_lpdwThreadID);
	SetThreadPriority(nPriority);
	m_bRunning = true;
	return m_hThread;
}

DWORD WINAPI GenericThread::ThreadProc(void * lpThis)
{
	((GenericThread *) lpThis)->run();
	return 0;
}

void GenericThread::run()
{

}

void GenericThread::SetThreadPriority(int nPriority)
{
	::SetThreadPriority(m_hThread, nPriority);
}

int GenericThread::GetThreadPriority()
{
	return ::GetThreadPriority(m_hThread);
}

bool GenericThread::IsThreadRunning()
{
	return m_bRunning;
}

void GenericThread::SuspendThread()
{
	::SuspendThread(m_hThread);
}

void GenericThread::ResumeThread()
{
	::ResumeThread(m_hThread);
}

void GenericThread::TerminateThread()
{
	CloseHandle(m_hThread);
	m_bRunning = false;
}

HANDLE GenericThread::GetThreadHandle()
{
	return m_hThread;
}

Share this post


Link to post
Share on other sites
Advertisement
A static member function operates independently to any instance of a class. There's no implicit "this" argument. So as far as calling conventions go, it doesn't look any different to a regular C function.

However, since a static member function is indeed a member, it can access non-public members of instances of the class it belongs to.

This is why the pass a pointer-to-a-static-method is so common when writing thread classes.

Because a static member function has no "this" pointer available to it, data has to be passed in via the opaque parameter slot that CreateThread provides.

Other stuff:

0. Don't do this yourself, unless it's just for fun. There's already a defacto-standard C++ threading interface.

1. Make your thread class un-copyable. This is usually done by declaring a private copy constructor and assignment operator.

2. Don't terminate threads. In general, it's really bad. Your destructors won't run, resources will leak (including locks... deadlock ahoy). Instead, ask them to stop what they're doing and wait for them to finish.

3. Priority and suspend/resume functionality are almost always useless. I'd also have the threads you create "inherit" the stack size from their parent.

4. I think _beginthread[ex] is generally preferred to CreateThread as it interacts better with the C runtime library.

Share this post


Link to post
Share on other sites
Thank you for these hand tips.

Quote:
Original post by the_edd
0. Don't do this yourself, unless it's just for fun. There's already a defacto-standard C++ threading interface.
Actually, one of the main reasons is fun. I want to use my own code.

Quote:
Original post by the_edd
1. Make your thread class un-copyable. This is usually done by declaring a private copy constructor and assignment operator.
If I use the singleton pattern I won't be able to create more than one instance of my class. I want to this class in big project, I need more than one instances of it.

Quote:
Original post by the_edd
2. Don't terminate threads. In general, it's really bad. Your destructors won't run, resources will leak (including locks... deadlock ahoy). Instead, ask them to stop what they're doing and wait for them to finish.
Yeah, this is quite important, I am going to modify the code to prevent these possible leaks.

Quote:
Original post by the_edd
3. Priority and suspend/resume functionality are almost always useless. I'd also have the threads you create "inherit" the stack size from their parent.
If so, that's bad.

Quote:
Original post by the_edd
4. I think _beginthread[ex] is generally preferred to CreateThread as it interacts better with the C runtime library.
I saw that some of the examples are using _beginthread() rather than CreateThread(). But since I won't be using any C runtime libraries in my code, I think it will be just fine using CreateThread().

Share this post


Link to post
Share on other sites
Quote:
Original post by Battousai
Quote:
Original post by the_edd
1. Make your thread class un-copyable. This is usually done by declaring a private copy constructor and assignment operator.
If I use the singleton pattern I won't be able to create more than one instance of my class. I want to this class in big project, I need more than one instances of it.

Preventing copying is not the same thing as preventing multiple instances. With a private copy constructor and assignment operator, you cannot make copies of the objects. But there is nothing with it that prevents multiple instances to be created.

Share this post


Link to post
Share on other sites
What is the code for your "main" function that is creating the threads.
Considering I don't see any code there to wait for the thread to finish i have this advice:
Remember you need to wait in your main for the thread objects to finish, or you will get a crash! Expecially the way you have your destructors made up.
Thus it could be a fluke of timing that your program doesn't crash with only one thread.

Share this post


Link to post
Share on other sites
Quote:
Original post by KulSeran
What is the code for your "main" function that is creating the threads.
Considering I don't see any code there to wait for the thread to finish i have this advice:
Remember you need to wait in your main for the thread objects to finish, or you will get a crash! Expecially the way you have your destructors made up.
Thus it could be a fluke of timing that your program doesn't crash with only one thread.


main.cpp
#include <QtCore/QCoreApplication>
#include "TestClassA.h"
#include "TestClassB.h"

int main(int argc, char *argv[])
{
QCoreApplication application(argc, argv);

TestClassA a;
//TestClassB b;

a.SetThreadPriority(-15);
//b.SetThreadPriority(+15);

a.StartThread();
//b.StartThread();


return application.exec();
}

I crashes if I uncomment those three lines above.
I used QT toolkit, so the program doesn't end up immediately.

TestClassA.cpp
#include "TestClassA.h"

TestClassA::TestClassA(void)
{
}

TestClassA::~TestClassA(void)
{
}

void TestClassA::run()
{
for (int i=0; i<=20; i++) std::cout << "Fat cat ate the rat." << std::endl;
}


TestClassA.h
#pragma once
#include "GenericThread.h"
#include <string>
#include <iostream>

class TestClassA : public GenericThread
{
public:
TestClassA();
~TestClassA();

private:
void run();
};


"TestClassB" is exactly same with "TestClassA", except that it print a different string.

Share this post


Link to post
Share on other sites
You are invoking CreateThread incorrectly. The last argument is an output parameter - a (optional) pointer to a DWORD where the thread ID will be stored.

In your code, you're passing in m_lpdwThreadID, which is an uninitialized pointer. You're getting lucky the first the time call is made, but not so much on the second call (undefined behaviour, bets are off etc ;])

Instead, define an actual DWORD member variable for the thread ID like so:

// The thread id assigned by Windows, there is no use for now
DWORD m_dwThreadID;



.. and call CreateThread with a pointer to this variable:

m_hThread = CreateThread(NULL, stack_size, (LPTHREAD_START_ROUTINE) ThreadProc, (void *) this, dwCreationFlags, &m_dwThreadID);

Share this post


Link to post
Share on other sites
Quote:
Original post by mattd
You are invoking CreateThread incorrectly. The last argument is an output parameter - a (optional) pointer to a DWORD where the thread ID will be stored.

In your code, you're passing in m_lpdwThreadID, which is an uninitialized pointer. You're getting lucky the first the time call is made, but not so much on the second call (undefined behaviour, bets are off etc ;])

Instead, define an actual DWORD member variable for the thread ID like so:
*** Source Snippet Removed ***

.. and call CreateThread with a pointer to this variable:
*** Source Snippet Removed ***


Wow, you were write, now my thread class is working just fine. I declared two objects from it, didn't crash and run both two threads simultanously.


Quote:
Original post by KulSeran
Remember you need to wait in your main for the thread objects to finish, or you will get a crash!


Now, I want to add a join() method to my class, any ideas how?

Share this post


Link to post
Share on other sites
Note 1. Since your threads aren't really using any data from your program, that explains why you don't see a crash from not waiting. Once you have a join function,
you should add code to the thread class destructor to join the thread and then close the handle. That way the thread object can't be destructed while the thread itself is running.

To "join" a thread, you want to look into adding a call to WaitForSingleObject( m_hThread )
. It will lock the calling thread untill the m_hThread handle is unlocked, which will only happen when the thread terminates. If the thread terminated long before your join call, then the wait call should immediately return. If the thread is locked up for some reason (actual lockup, waiting on some other mutex, or suspended), then the call will never return, so you may want to add a "try_join" type call that calls wait with a timeout.


If you want to synchronize with the thread for reasons other than the thread terminating, either create a mutex, or a critical section.

Share this post


Link to post
Share on other sites
Quote:
Original post by KulSeran
Note 1. Since your threads aren't really using any data from your program, that explains why you don't see a crash from not waiting. Once you have a join function,
you should add code to the thread class destructor to join the thread and then close the handle. That way the thread object can't be destructed while the thread itself is running.

To "join" a thread, you want to look into adding a call to WaitForSingleObject( m_hThread )
. It will lock the calling thread untill the m_hThread handle is unlocked, which will only happen when the thread terminates. If the thread terminated long before your join call, then the wait call should immediately return. If the thread is locked up for some reason (actual lockup, waiting on some other mutex, or suspended), then the call will never return, so you may want to add a "try_join" type call that calls wait with a timeout.


If you want to synchronize with the thread for reasons other than the thread terminating, either create a mutex, or a critical section.


Thank you very much for your helps, now I think my simple thread class is complete:

GenericThread.h
#ifndef _GENERIC_THREAD_H_
#define _GENERIC_THREAD_H_

#include <Windows.h>

class GenericThread
{
public:
GenericThread();
virtual ~GenericThread();

// Create and run the thread
HANDLE StartThread(int nPriority = 0, bool create_suspended = false, SIZE_T stack_size = 0, bool reserve_stack = false);

// This member function will run when you call StartThread()
static DWORD WINAPI ThreadProc(void * lpParameter);

// The run() method which will be called for threading
virtual void run();

// -15 : lowest; +15 : highest
void SetThreadPriority(int nPriority);

// Get currently assigned priority level
int GetThreadPriority();

// Get thread status
bool IsThreadRunning();

// WaitForSingleObject()
bool JoinThread();

// Make sure that the thread has joined
bool IsThreadJoined();

// Pause execution
void SuspendThread();

// Resume execution
void ResumeThread();

// Terminate execution
void TerminateThread();

// Return thread handle
HANDLE GetThreadHandle();

private:

// Handle to the thread
HANDLE m_hThread;

// The thread id assigned by Windows, there is no use for now
DWORD m_dwThreadID;

// Thread status
bool m_bRunning;

// Thread joined or not
bool m_bJoined;
};

#endif


GenericThread.cpp
#include "GenericThread.h"

GenericThread::GenericThread()
{
m_bRunning = false;
m_bJoined = false;
}

GenericThread::~GenericThread()
{
JoinThread();
if (m_bRunning) CloseHandle(m_hThread);
}

HANDLE GenericThread::StartThread(int nPriority /*= 0*/, bool create_suspended /*= false*/, SIZE_T stack_size /*= 0*/, bool reserve_stack /*= false*/)
{
DWORD dwCreationFlags = 0;
if (create_suspended) dwCreationFlags |= CREATE_SUSPENDED;
//if (reserve_stack) dwCreationFlags |= STACK_SIZE_PARAM_IS_A_RESERVATION;
m_hThread = CreateThread(NULL, stack_size, (LPTHREAD_START_ROUTINE) ThreadProc, (void *) this, dwCreationFlags, &m_dwThreadID);
SetThreadPriority(nPriority);
m_bRunning = true;
return m_hThread;
}

DWORD WINAPI GenericThread::ThreadProc(void * lpThis)
{
((GenericThread *) lpThis)->run();
return 0;
}

void GenericThread::run()
{

}

void GenericThread::SetThreadPriority(int nPriority)
{
::SetThreadPriority(m_hThread, nPriority);
}

int GenericThread::GetThreadPriority()
{
return ::GetThreadPriority(m_hThread);
}

bool GenericThread::IsThreadRunning()
{
return m_bRunning;
}

bool GenericThread::IsThreadJoined()
{
return m_bJoined;
}

void GenericThread::SuspendThread()
{
::SuspendThread(m_hThread);
}

void GenericThread::ResumeThread()
{
::ResumeThread(m_hThread);
}

bool GenericThread::JoinThread()
{
if (!m_bJoined)
if (WaitForSingleObject(m_hThread, INFINITE) == WAIT_FAILED)
return m_bJoined = false;
else
return m_bJoined = true;
return m_bJoined;
}

void GenericThread::TerminateThread()
{
CloseHandle(m_hThread);
m_bRunning = false;
}

HANDLE GenericThread::GetThreadHandle()
{
return m_hThread;
}


Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!