Need help with mutex

auralius manurung

I have this class:

class CPROXY
{
public:

CPROXY(void);
~CPROXY(void);
int CreateConnector();
int CreateAcceptor(int nPort);
:
:
    :

private:

ACE\_SOCK\_Acceptor	client\_acceptor;
ACE\_SOCK\_Connector	server\_connector;

**std::list <spair\*>	queue;**

}

Now the implementation is like this:

int CPROXY::ServerThreadRunner()
{
:
DWORD thid;
HANDLE hServerThread = CreateThread(NULL, 0, ServerThread, this, 0, &thid);
:
:
}

DWORD WINAPI CPROXY::ServerThread(LPVOID param)
{
CPROXY *newobj = (CPROXY *)param;

newobj->CreateConnector();

return 0;

}

int CPROXY::CreateConnector()
{
EnterCriticalSection(&guard);
SPAIR *sPair = queue.front();
queue.pop_front();
LeaveCriticalSection(&guard);

SetEvent(wait\_server);
:
:
    :

Take a look at queue. Queue is declared as a list of SPAIR. Do you think it is necessary for me to guard it with critical section every time is accessed by a thread? I mean, before i run the thread i did put it into a newobj, so meaning that every newobj has its own member so no need for me to put critical section to guard it, right? PLEASE, correct me if I'm wrong! i need to optimize this code.

Mark Salsbery

auralius wrote:

before i run the thread i did put it into a newobj, so meaning that every newobj has its own member

I'm not sure what you mean by that. Your code doesn't show how many CPROXY objects are created. Just because you do this:

CPROXY *newobj = (CPROXY *)param;

doesn't mean there's a new CPROXY object - it's just a pointer to an object that is passed to the thread. If every thread has its own CPROXY object, then no, you don't need synchronization. If every thread is using the same CPROXY object, and there's any chance more than one thread can access this object at the same time, then yes you need synchronization. Mark

Mark Salsbery Microsoft MVP - Visual C++ :java:

auralius manurung

Thank for your reply. I mean there is a main function that will call int ServerThreadRunner(). Meaning that for every thread will be handled by new CPROXY newobj pointer. So, if it is only pointer pointing the same location, does it mean that i need to put synchronization for every data member that accessed by the thread function? How about this: std::list <spair> queue; vs std::list <spair*> queue; which one needs synchronization, and which one doesn't need? What came to my mind is: there is no need for synchronization unless i put the queue as a static variable. am i right? static std::list <spair*> queue; sorry if my question is not really good. i hope you understand. i can't just solve that problem by trial & error because both of them work. I need strong concept.

Mark Salsbery

auralius wrote:

eaning that for every thread will be handled by new CPROXY newobj pointer.

Different pointers to CPROXY objects is not the same as different CPROXY objects. All the threads will have their own pointers but they could all point to the same object, in which case you'd need synchronization. If there's a separate CPROXY object for every thread, then no synchronization is necessary.

auralius wrote:

there is a main function that will call int ServerThreadRunner()

Can you show how you do that?

auralius wrote:

What came to my mind is: there is no need for synchronization unless i put the queue as a static variable. am i right?

No. It all comes down to actual object instances. Multiple threads can access a single object instance no matter how it's stored. If multiple threads can change the same object, then that object should be protected with synchronization. You haven't shown relevant code so it's hard to tell if you need it there or not. Mark

Mark Salsbery Microsoft MVP - Visual C++ :java:

auralius manurung

class CPROXY
{
public:

CPROXY(void);
~CPROXY(void);

int Run(int nPort);

static DWORD WINAPI MotherThread(LPVOID param);
static DWORD WINAPI DaughterThread(LPVOID param);
int MotherThreadWorker();
int DaughterThreadWorker();

int GetAddressAndPort(char \* cStr, char \*cAddress, int \* nPort);

private:

ACE\_SOCK\_Acceptor	client\_acceptor;
ACE\_SOCK\_Connector	server\_connector;

CRITICAL\_SECTION	guard;
HANDLE			wait;

bool				isMotherThreadRunning;
bool				isDaughterThreadRunning;

**static std::list<ACE\_SOCK\_Stream> queue;**

};

The implementation is like this:

CPROXY::CPROXY(void)
{
InitializeCriticalSection(&guard);
wait = CreateEvent(NULL, FALSE, FALSE, NULL);
}

CPROXY::~CPROXY(void)
{
}

std::list<ACE_SOCK_Stream> CPROXY::queue;

int CPROXY::Run(int nPort)
{
DWORD thid;
HANDLE hMotherThread = CreateThread(NULL, 0, MotherThread, this, 0, &thid);
if (!hMotherThread)
return -1;

ACE\_SOCK\_Stream client\_stream;
ACE\_INET\_Addr addr;
addr.set(nPort, addr.get\_ip\_address());

int e = client\_acceptor.open(addr);
if (e == INVALID\_SOCKET)
	return -1;

while(true)
{
	int e = client\_acceptor.accept(client\_stream);
	if (e == INVALID\_SOCKET)
		continue;

	//Store in a buffer.
	**EnterCriticalSection(&guard);
	queue.push\_back(client\_stream);
	LeaveCriticalSection(&guard);**
}

return 0;

}

DWORD WINAPI CPROXY::MotherThread(LPVOID param)
{
CPROXY *newobj = (CPROXY *)param;

newobj->MotherThreadWorker();

return 0;

}

int CPROXY::MotherThreadWorker()
{
isMotherThreadRunning = true;

while (isMotherThreadRunning)
{
	**EnterCriticalSection(&guard);
	bool isEmpty = queue.empty();
	LeaveCriticalSection(&guard)**;

	if (!isEmpty){
		DWORD thid;
		**HANDLE hDaughterThread = CreateThread(NULL, 0, DaughterThread, this, 0, &thid);**
		if (!hDaughterThread)
			continue;

		printf("\\nWAITING!\\n");
		WaitForSingleObject(wait, INFINITE);
		printf("\\nFINISHED!\\n");
	}
}
return 0;

}

DWORD WINAPI CPROXY::DaughterThread(LPVOID param)
{
CPROXY *newobj = (CPROXY *)param;

newobj->DaughterThreadWorker();
    :
    :
return 0;

}

int CPROXY::DaughterThreadWorker()
{
char buf[BUFSIZE];
char cServerAddress[256];
int nServerPort;

**EnterCriticalSection(&guard);
ACE\_SOCK\_Stream client\_stream = queue.front();
queue.pop\_front();
LeaveCriticalSection(&guard);**

SetEvent(wait);

Mark Salsbery

auralius wrote:

Does it still need synchronization?

Yes. Based on the code shown (if I read it correctly) you always have the possibility of two threads simultaneously accessing the one queue - either the main thread and the MotherThread or the main thread and the DaughterThread. There's only one queue object because there's only one CPROXY object, so it doesn't make any difference where/how the one object is allocated. Mark

Mark Salsbery Microsoft MVP - Visual C++ :java:

modified on Wednesday, October 22, 2008 12:59 AM

auralius manurung · modified on Wednesday, October 22, 2008 12:59 AM

Thank you very much for your help... i think i need to learn more and more... :)

Mark Salsbery

You're welcome! :)

Mark Salsbery Microsoft MVP - Visual C++ :java: