Click here to Skip to main content
15,901,205 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
Hi.

I am writing a Small Server app, which uses TCP/IP. In the classical style, i wait for 'WSAAccept' to unblock, and start a New thread, to process the Client Socket received.
I also use a Structure, to pass information on to the thread function.
Now, the Parent Thread may Loop again (and Modify the Structure to which 'CreateThread' got a pointer before the Thread Function has had a chance to take the values that the Param pointed at.

Below is a Code Snippet.

C++
struct THR_INFO{
	CSgSrvrCore* pThis;
	SOCKET hSocket;
};


typedef THR_INFO* LP_THR_INFO;

void Class::Function(){

	// Other Code

	THR_INFO* pThrInfo;

	DWORD dwThreadID;
	THR_INFO* pThrInfo;

	#ifdef PARANOID
		pThrInfo=new THR_INFO;
		pThrInfo->pThis=this;
		pThrInfo->hSocket=h_sockClient;
	#else

		THR_INFO ThrInfo={this,h_sockClient};
		pThrInfo=&ThrInfo;
	#endif

	CreateThread(0, 0, ConnectToClientThread, pThrInfo, 0, &dwThreadID);


	//Other Code
}

//static 
void Class::ConnectToClientThread(void* pParam){

	LP_THR_INFO pThrInfo=(LP_THR_INFO)p_Param;
	SOCKET h_sockClient=pThrInfo->hSocket;
	Class* pThis=pThrInfo->pThis;
	ASSERT(pThis);

	#ifdef PARANOID
         // The Item was Created with 'new'
		delete p_Param;
	#endif


}



Should I not be PARANOIDE, and the basic function will be thead safe, or should I include further Synchronisation Objects.

Regards :)
Posted
Comments
Nelek 15-Apr-12 19:42pm    
Paranoia is good without abusing. As far as it doesn't affect performance or other aspects, I would say: if not sure... the more sugar, the sweeter.
[no name] 15-Apr-12 23:02pm    
"Now, the Parent Thread may Loop again (and Modify the Structure to which 'CreateThread' got a pointer before the Thread Function has had a chance to take the values that the Param pointed at."

I don't understand this. With Paranoid on don't you instantiate a new structure object on the heap prior to each call to CreateThread. Thus there should be a 1-1 association between threads and structures. You show the thread then deleting the structure object. This would seem to be the whole point of the exercise.
Aescleal 16-Apr-12 5:44am    
Be careful advising more synchronisation - you can cause all sorts of deadlocks if you don't know what you're doing. Actually most people introduce deadlocks when they know what their doing but that's another story.

1 solution

[edited to correct the fact that I misread something in the original code]

From the looks of it, it's "thread safe" if you turn on PARANOID

You are, in reality, passing a pointer by value to the created thread. That essentially creates a copy of the pointer on the local stack of the created thread so, in theory, any modification to the pointer itself in the main code (you do another "new") should not affect the copy owned by any thread (pParam). Now, that said, the "structure" that the pointer points to is, of course, something that you shouldn't modify once you pass the pointer to the created thread. Initialize the structure and then let it go. And you should "delete" the object when you are done with it. The main thread did the "new" and passed ownership / responsbility for the structure to the created thread. That thread should clean up after itself to avoid memory leaks.

With PARANOID turned off, you are instantiating your ThrInfo structure on the local stack of the "Function()" and passing the pointer to that. Local Stack structures can be temporary and go out of scope when the function exits so your thread has a pointer to some stack memory that may be overwritten by the main thread's further execution.

Cardinal Rule of Threading: Do not pass pointers to local stack objects, you will regret it.
 
Share this answer
 
v2
Comments
Bram van Kampen 15-Apr-12 20:44pm    
Thanks,
With PARANOID turned on I thought I created a Pointer on the Local Stack, pointing to a Global Object This pointer is passed to the thread's stack, but still points to the same global object. I take it that all that is done when 'CreateThread' returns. MS Documentation states specfically that 'CreateThread' may return before the Newly Started thread completes( Otherwise 'CreateThread' would be pointless)or even before it commences!

I had Bad feelings about the code without 'PARANOIDE', and you confirm my bad feelings about it. The problem with synchronisation objects is, that I would have to apply the Lock in the Parent thread, and release it from the Child Thread. This is Something that does not sound right. (Having said that, maybe it is perfectly acceptable).

Apart from that, what I'm trying to do is not exactly breaking the mould. Useage of the 'CreateThread' function is peppered through all MS Apps.
I can only assume that they made it 'Usefull'. Unless if anyone comes up with a different Idea, I will for now assume that You're right in considering the 'PARANOIDE' option as threadsafe.

Thanks;

Regards :)
Chuck O'Toole 15-Apr-12 21:00pm    
Well, to clarify your thoughts about CreateProcess(). You need to consider what you are passing as the argument. Consider the difference between passing "pThrInfo" and "&pThrInfo". In the first instance, you are passing the "contents" of "pThrInfo", in the second you would be passing the address of "pThrInfo". So, when CreateProcess() creates the argument list to pass to the created thread, it's going to put the argument on the local stack of the created process for the initial "call" to the thread. Passing the "contents" of the pointer, as you do in your code, removes any concern you have about the original pointer in the caller's code. If you passed the "address of the pointer" you would still rely on the pointer in the caller's code being "safe" until you did something with it in the created thread. Depending on the contents of the caller's variable requires synchronization objects.
JackDingler 16-Apr-12 18:06pm    
As they are pointing out.

If you want to pass different data to each thread, then create a new instance of THR_INFO each time. Set up it's data. Then pass the pointer to the thread and let the thread delete it. In this model each thread will have it's own safe copy of the structure to read, with no fear it will randomly change.

If you pass the same pointer to every thread, and change the data it points to each time, then the data the thread will use, will be unpredictable.

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



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900