• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
codejockey

How to thread a class - Client Server Program

12 posts in this topic

So I've looked around with Bing search as to how to thread a class.  I see lot's of nice wrappers with lots of heavy duty pointer stashing and caching and said there's got to be an easier way.  Well here it is, what you all have been waiting for ... threading a class!!  It's so simple your just going to want to do it in every class you write.  I'll just put my server class here and comment where the thread dependent items are.  (btw, it's still a work-in-process but it does get a listen server and a client connection with data being passed)  The includes are just windows stuff and some simple buffers and not necessary to discuss.
 
Header

 

#pragma once

#include "includes.h"
#include "ChatPacket.h"

class Server
{
public:

	Server(void);
	~Server(void);

	// some normal methods
	short Server::Initialize(short port);
	short Server::Disconnect();
	short Server::Send();
	void Server::Listening();

	// these two methods are going to be the threads
	// notice the use of static and the (void* t) parameter
	static void Server::Accepting(void* t);
	static void Server::Receiver(void* t);

	char buffer[VERY_LARGE_STRING];

private:

	WSADATA wsaData;

	SOCKET listenSocket;
	SOCKET clientSocket;

	// some Booleans to keep track of which threads are active
	bool listenerIsActive;
	bool accepterIsActive;
	bool receiverIsActive;

	int receiveLength;
	int sendLength;

	// thread ID variables
	uintptr_t listenThread;
	uintptr_t acceptThread;
	uintptr_t receiveThread;
	uintptr_t sendThread;

	struct addrinfo *result, *ptr, hints;

	ChatPacket* chatPacket;
};

Methods

 

#include "Server.h"

Server::Server(void)
{
	memset(this, 0x00, sizeof(Server));
}

Server::~Server(void)
{
	if (receiverIsActive)
	{
		//set the Boolean to false so the thread will stop
		receiverIsActive = false;
		// wait for 1 second so the thread can stop
		Sleep(1000);
	}

	if(accepterIsActive)
	{
		// and again
		accepterIsActive = false;
		Sleep(1000);
		closesocket(clientSocket);
		clientSocket = 0;
	}

	if(listenerIsActive)
	{
		// and again for this one
		listenerIsActive = false;
		Sleep(1000);
		closesocket(listenSocket);
		listenSocket = 0;
	}

	WSACleanup();
}

short Server::Initialize(short port)
{
	// initialize winsock
	r = WSAStartup(MAKEWORD(2, 2), &wsaData);
	if (FAILED(r))
	{
		PrintDebugString("%s", DXGetErrorString(r));
		return 0;
	}

	memset(&hints, 0x00, sizeof(hints));
	hints.ai_family = AF_INET;
	hints.ai_socktype = SOCK_STREAM;
	hints.ai_protocol = IPPROTO_TCP;
	hints.ai_flags = AI_PASSIVE;

	// resolve this servers address and port
	// port has to be a character string ... who'da  thought that!
	char p[6];
	sprintf(p, "%i", port);

	r = getaddrinfo(NULL, p, &hints, &result);
	if (FAILED(r))
	{
		PrintDebugString("%s", DXGetErrorString(r));
		WSACleanup();
		return 0;
	}

	// create a SOCKET for this server
	listenSocket = socket(result->ai_family, result->ai_socktype, result->ai_protocol);

	if (listenSocket == INVALID_SOCKET)
	{
		freeaddrinfo(result);
		WSACleanup();
		listenSocket = 0;
		return 0;
	}

	// bind the TCP listening socket
	r = bind(listenSocket, result->ai_addr, (int)result->ai_addrlen);
	if (r == INVALID_SOCKET)
	{
		freeaddrinfo(result);
		closesocket(listenSocket);
		WSACleanup();
		listenSocket = 0;
		return 0;
	}

	freeaddrinfo(result);

	// start the listen server for connect requests
	Server::Listening();

	// start the accepting thread for connect requests
	acceptThread = _beginthread(&Server::Accepting, 0, this);

	// start the receiving thread for receiving data
	receiveThread = _beginthread(&Server::Receiver, 0, this);

	return 1;
}

void Server::Listening()
{
	// thought this might need to be threaded but turns out it doesn't
	// so toss the boolean out
	listenerIsActive = true;
	r = listen(listenSocket, SOMAXCONN);
	if (r == INVALID_SOCKET)
	{
		closesocket(listenSocket);
		WSACleanup();
		listenSocket = 0;
	}
}

void Server::Accepting(void* t)
{
	// using this as a thread must cast the this pointer
	// to something usable
	Server* s = (Server*)t;

	// set the Boolean to true so we know the thread is running
	s->accepterIsActive = true;

	// infinite loop until the Boolean is changed to false somewhere else in the class
	// i.e the deconstructor or the disconnect method
	while(s->accepterIsActive)
	{
		s->clientSocket = accept(s->listenSocket, 0, 0);
		if (s->clientSocket == INVALID_SOCKET)
		{
			closesocket(s->clientSocket);
			WSACleanup();
			s->clientSocket = 0;
			return;
		}
		// needed a spot to put a stopper for my debugger
		if (s->clientSocket > 0)
		{
			int zz = 0;
			zz++;  //  I put the stopper on this line
		}
	}
}

short Server::Send()
{
	// note-to-self don't use strlen(buffer) as there could be 0x00 somewhere in the buffer
	// and will not send the entire buffer if there is
	sendLength = send(clientSocket, buffer, strlen(buffer), 0);
	if (sendLength == SOCKET_ERROR)
	{
		PrintDebugString("Server::Send failed with error: %d", WSAGetLastError());
		closesocket(clientSocket);
		WSACleanup();
		return 0;
	}
	
	return 1;
}

void Server::Receiver(void* t)
{
	Server* s = (Server*)t;

	// set the Boolean to show the thread is running 
	s->receiverIsActive = true;

	// loop until this Boolean is false
	while(s->receiverIsActive)
	{
		if (s->clientSocket > 0)
		{
			memset(s->buffer, 0x00, VERY_LARGE_STRING);
			s->receiveLength = recv(s->clientSocket, s->buffer, VERY_LARGE_STRING, 0);
			if (s->receiveLength > 0)
			{
				// doing my client receive logic here
				PrintDebugString("Server::Bytes received: %d", s->receiveLength);
				if ((s->buffer[0] == 'C') && (s->buffer[1] == 'H'))
				{				
					s->chatPacket = new ChatPacket();
					memcpy(s->chatPacket, s->buffer, s->receiveLength);
					PrintDebugString("Server::CH: %s", s->chatPacket->message);
					delete s->chatPacket;
					s->chatPacket = 0;
				}
				memset(s->buffer, 0x00, VERY_LARGE_STRING);
			}
			else if (s->receiveLength == 0)
			{
				PrintDebugString("Server::No Data...");
			}
			else
			{
				PrintDebugString("Server::Recv failed: %d", WSAGetLastError());
				closesocket(s->clientSocket);
				WSACleanup();
				s->receiverIsActive = false;
				return;
			}
		}
	}
}

short Server::Disconnect()
{
	receiverIsActive = false;

	if(accepterIsActive)
	{
		accepterIsActive = false;
		Sleep(1000);
		closesocket(clientSocket);
		clientSocket = 0;
	}

	if(listenerIsActive)
	{
		listenerIsActive = false;
		Sleep(1000);
		closesocket(listenSocket);
		listenSocket = 0;
	}
	
	WSACleanup();

	return 1;
}

And that's all there is to threading with a class.  Don't bother with starting the thread outside of the class.  Too much chaos in doing that.  Simply call a method within the class to start the threading or do it in the class on its own with the constructor.  One final thought is that this server is set up to connect only one client so don't just cut and paste and assume its going to handle multiple clients (hint: s->clientSocket[x] or maybe put them in a link list?)

 

Microsoft's MSDN Links for Client Server Programming with Winsock

 

Getting Started With Winsock

 

Complete Server Code

 

Complete Client Code

-4

Share this post


Link to post
Share on other sites
Some things that annoyed me without even starting to dig deeper:
1) using void as sole function parameter. You are using classes, that means C++ from the start. C calling conventions go out the window as step 1.
2) memset(this, ...). I don't care if you made sure this does not trigger undefined behavior. It's a horrible mess and the moment you need any polymorphism or any non-POD data member you suddenly have things blow up.
3) you do not prevent accidental copying of the Server class (private copy constructor/assignment operator, boost::noncopyable, = deleted declaration in C++11).

If I wanted threading I'd either go with boost::thread or std::thread (depending on preference and available C++11 capability). boost::asio is worth a look too. Unless you absolutely have to roll your own threading wrapper, I would strongly suggest not to (unless it is purely for educational purposes and never intended to be used in anything related to production code), especially at the level you appear to be at.
1

Share this post


Link to post
Share on other sites

Further flaws on the multi-threading and on the networking levels: A thread solely for accepting a connection? The main thread becomes blocked by listening for connections? An active wait synchronization between Receiver and Accepting? The client socket stored as member of the server object (also for demonstration purposes a bit strange)? No joining with terminated child threads?

 

It's so simple your just going to want to do it in every class you write.

Nonsense! Doing things just because they are possible is a No-Go. Resources would be wasted, and glitches accepted without any need. Keeping things as complex as needed but as simple as possible is the way to go.

Edited by haegarr
2

Share this post


Link to post
Share on other sites

Ok with that being said.  Rewrite it in your way (going to have to assume that's the right way) so everyone can be clear on how it's supposed to be done.

 

If you know assembler then you'll know this is fine:

memset(this, 0x00, sizeof(Server));

 

Perhaps constructive criticism instead of this type of verbosity:

VERY_LARGE_RETARDATION

 

Did you guys even read the source in the class?  blink.png 

-2

Share this post


Link to post
Share on other sites

This is getting annoying.  I only posted this because it shows how you can define the methods in a class to be threaded.  If you want it to be professionally used then I suggest doing all the changes that all these brilliant people pointed out to make it universally acceptable in the C++ programming community.  For me, it works, it does what I need, and isn't that actually what is important.  btw I wouldn't work anywhere that has C++ as its main programming language.  Get current with some managed code such as C# XNA or java.

-5

Share this post


Link to post
Share on other sites

codejockey, hodgman just used his own free time to write a detailed post explaining how you could improve your code. 

 

He's an excellent developer and you would do well to heed his advice instead of whining like a sulky teenager.

2

Share this post


Link to post
Share on other sites

This is getting annoying.  I only posted this because it shows how you can define the methods in a class to be threaded.  If you want it to be professionally used then I suggest doing all the changes that all these brilliant people pointed out to make it universally acceptable in the C++ programming community.  For me, it works, it does what I need, and isn't that actually what is important.  btw I wouldn't work anywhere that has C++ as its main programming language.  Get current with some managed code such as C# XNA or java.

 

If you don't use thread synchronisation and memory barriers or fences you have a very real potential to generate race conditions, live or dead locks in your code. Only on very specific conditions can you omit a these synchronisation primitives, eg when you definitely know that the thread will only write to a certain part of the memory that no one else can touch at that point. This is generally not the case and to be thread safe you need fences/barriers or synchronisation primitives to safe guard this, as there is nothing stopping the compiler for seriously messing with the instruction order.

 

A fence, barrier or synchronisation primitive forces the compiler to not move any instruction before an acquire operation below that aqcuire, and for a release it guarantees that nothing will reorder up from below or out from in between the acquire and release. There is a whole lot of optimisation going on even after the compiler is done reordering stuff, out of order CPU's can also mess with this instruction order and here these promitives will guarantee the CPU will do the same thing as the compiler around them.

 

Concurence is one of the hardest problems to solve in CS and as soon as you go beyond the trivial example it becomes extremely complex.
 

Even in managed code you need these operations to guarantee all of what has been mentioned in this thread about thread safety and correct operation of a multi-threaded application.

 

Threading an application comes with a very big warning that you need to be extremely careful with what you are doing especially around shared memory locations.

Edited by NightCreature83
1

Share this post


Link to post
Share on other sites


I wouldn't work anywhere that has C++ as its main programming language.  Get current with some managed code such as C# XNA or java.

 

Are you just trolling now? If you wouldn't use C++, why are you sharing C++ code with the rest of us?

2

Share this post


Link to post
Share on other sites

This is getting annoying.  I only posted this because it shows how you can define the methods in a class to be threaded.  If you want it to be professionally used then I suggest doing all the changes that all these brilliant people pointed out to make it universally acceptable in the C++ programming community.  For me, it works, it does what I need, and isn't that actually what is important.  btw I wouldn't work anywhere that has C++ as its main programming language.  Get current with some managed code such as C# XNA or java.

But the code doesn't define methods to be threaded. It works on your particular machine, on your particular compiler settings, for the tests you did on it, but it's not correct code. Sharing memory without synchronization primitives is undefined behavior in C++, also called "catch fire semantics" because the code is legally allowed by the C++ standard to do anything including make your computer catch fire. Furthermore, this code is the worst kind of erroneous behavior, because it works most of the time, and doesn't give an easy to trace error when it doesn't, making the errors very hard to track down.
0

Share this post


Link to post
Share on other sites

If you know assembler then you'll know this is fine:

memset(this, 0x00, sizeof(Server));

 

It has nothing to do with assembler. And no, that's the most atrocious, evil piece of code I've ever seen and belongs in the "Coding Horrors" section of this forum.

 

The correct thing to do is use an initialiser list in the constructor:

Server::Server() :
    listenerIsActive( false ),
    accepterIsActive( false ),
    receiverIsActive( false ),
    receiveLength( 0 ),
    sendLength( 0 )
    /* -- etc -- */
{
}
Edited by TheComet
0

Share this post


Link to post
Share on other sites
I wouldn't call it just that. For example, by adding the simple line
static_assert(std::is_pod<Server>::value);
would probably make it safe. I still wouldn't do it myself, but the compiler should scream at you instead of silently setting fire to the system.

My problem was less with the memset as such as more with the complete thoughtlessness and ignorance in putting it there. If you absolutely have to put something like that in there, at least comment the inherent dangers properly. If possible (not without C++11 support) also add compile time check. Edited by BitMaster
0

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0