Pthread mutex issue
Author
Hi, All,
Any help with this problem would be greatly appreciated! I’m building a server that contains two pthreads, one for accepting new connections and the other one for handling messages. They both work off a common collection of socket descriptors (stored in a vector). Basically, the Accept thread adds new elements to the collection while the message thread iterates through the collection to reference each element of sockets. Because they both access a shared object I use pthread_mutexs to synchronize the threads.
My problem is when I make a call to phread_mutex_unlock I lose reference to the elements within that shared collection. This problem has baffled me for a couple of days now! Below is a small sample of the code.
//Server.h
Vector _vClients;
pthread_mutex_t _pMutex;
//Server.cpp
//Class Method – Message Thread Body
While(true)
{
pthread_mutex_lock(&_pMutex);
Recv(((SOCKET *)((CClientInfo*)(_vClients))->iSocket), msg);
pthread_mutex_unlock(&_pMutex);
}
Note: If I put a break point on Recv and add _vClients to my watch list I can see valid socket descriptors in _vClients (the first iteration). Once you step to the next line (unlock) the elements in _v Clients is lost and the next iteration call to Recv results in an “Memory Access Violation Error” which is the result of a bad pointer.
Thank you for your valuable time
Neil
This has nothing to do with networking.
Your Vector is probably re-allocating when the other thread modifies the vector, so any pointer value you''ve held on to is probably bad at that point.
By the way: the locking design you have is horrible. It contains no parallelism. You unlock the lock, and then immediately lock it again, to call Recv() again. There is no actual work overlap between the threads. You might as well go with select() and/or non-blocking sockets in a single thread, and avoid the threading pitfalls -- that''ll run faster, to boot.
Your Vector is probably re-allocating when the other thread modifies the vector, so any pointer value you''ve held on to is probably bad at that point.
By the way: the locking design you have is horrible. It contains no parallelism. You unlock the lock, and then immediately lock it again, to call Recv() again. There is no actual work overlap between the threads. You might as well go with select() and/or non-blocking sockets in a single thread, and avoid the threading pitfalls -- that''ll run faster, to boot.
Author
Hahah, that was just an example and non-blocking is not an option.
Thanks for your reply
Thanks for your reply
Hello Aphex3000,
Question are _vClients and _pMutex class members?
If not I would not declear and define the Vector in the header file.
Only declear an extern Vector _vClients;
Then in the source file define the vector.
You might have two different Vectors if you don''t do this and include the header file in more then one source file.
Same idea with pthread_mutex_t _pMutex;
only declear an extern pthread_mutex_t _pMutex; in the header
and define in source.
Next I would have to ask what is Vector?
Is it home made version of something like std::vector?
Is it a typedef for a std:vector?
If it based on std:vector then still should not have a problem.
If you are sure your using the same _pMutex and same _vClients between your two threads then you should not have this problem.
Are you sure you set up the mutex correctly?
ie.
called pthread_muteattr_init(...)
and then pthread_mutex_init(...)
Else, if not, you might not have a valid mutex.
If you use a static mutex, ie not calling above functions, then there should be a MACRO to set it up for default mutex attributes.
By the way what OS is this on?
Lord Bart
Question are _vClients and _pMutex class members?
If not I would not declear and define the Vector in the header file.
Only declear an extern Vector _vClients;
Then in the source file define the vector.
You might have two different Vectors if you don''t do this and include the header file in more then one source file.
Same idea with pthread_mutex_t _pMutex;
only declear an extern pthread_mutex_t _pMutex; in the header
and define in source.
Next I would have to ask what is Vector?
Is it home made version of something like std::vector?
Is it a typedef for a std:vector?
If it based on std:vector then still should not have a problem.
If you are sure your using the same _pMutex and same _vClients between your two threads then you should not have this problem.
Are you sure you set up the mutex correctly?
ie.
called pthread_muteattr_init(...)
and then pthread_mutex_init(...)
Else, if not, you might not have a valid mutex.
If you use a static mutex, ie not calling above functions, then there should be a MACRO to set it up for default mutex attributes.
By the way what OS is this on?
Lord Bart
hplus:
can you elaborate on how you would remedy the design? I think currently what Aphex is doing is hes setting up 2 threads: 1 is used to listen and put CClientInfo objects into a Vector, and the other thread is his message thread, which seems to receive the actual data off the sockets. Are you saying he should put the accept call, the read calls, and the write calls in one thread, and do away with the mutex entirely? this sounds more efficient, but more details are needed.
-neko
can you elaborate on how you would remedy the design? I think currently what Aphex is doing is hes setting up 2 threads: 1 is used to listen and put CClientInfo objects into a Vector, and the other thread is his message thread, which seems to receive the actual data off the sockets. Are you saying he should put the accept call, the read calls, and the write calls in one thread, and do away with the mutex entirely? this sounds more efficient, but more details are needed.
-neko
Yes, that''s what I''m saying. That''s what select() is for. Look for any tutorial on writing a single-threaded game server, and you''ll see how it''s done: stuff sockets into select(), pull out which sockets actually need attention, and service those sockets.
Typically, "servicing" means reading for connected sockets, and accepting for a listening socket.
An alternative would be to make a few more rules:
- the accept thread only creates socket objects, and pushes objects onto the list
- the reading thread only dereferences objects in the list, and removes objects, and deletes them once removed
That way, you only need to synchronize while looking at the list; you don''t need to hold the lock while going into the object. Thus, a threaded version might look like:
Still, this is less efficient, because it involves locking where no "real" parallelism will happen. Assuming much more time is spent cumulatively in doStuff() than in accept(), which is usually the case.
Typically, "servicing" means reading for connected sockets, and accepting for a listening socket.
An alternative would be to make a few more rules:
- the accept thread only creates socket objects, and pushes objects onto the list
- the reading thread only dereferences objects in the list, and removes objects, and deletes them once removed
That way, you only need to synchronize while looking at the list; you don''t need to hold the lock while going into the object. Thus, a threaded version might look like:
void thread_1() { forever() { Client * c = new Client( accept( listeningSocket ) ); lock(); vec.push_back( c ); unlock(); }}void thread_2() { forever() { int i = 0; lock(); if( i >= vec.size() ) { i = 0; } Client * c = vec.at( i ); unlock(); c->doStuff(); if( c->done() ) { lock(); vec.swapEntries( i, vec.size() ); vec.resize( vec.size()-1 ); unlock(); delete c; } else { ++i; } }} Still, this is less efficient, because it involves locking where no "real" parallelism will happen. Assuming much more time is spent cumulatively in doStuff() than in accept(), which is usually the case.
thanks hplus. it sounds like a single thread operating on all sockets is the prefferred model to use.
Heres another question...the architecture we just spoke about outlines how the server would operate...how would the client be setup? I''m assuming itd be just creating one socket, connecting to the server, and then reading/writing off that socket. But are sockets bidirectional and asynchronous? (for example can I read off of a socket and write off of that same socket in parallel, or is it only able to transport data in one direction at a time?) I suppose what Im getting at is do I need 2 sockets for client/server communication if I want to be able to send data and receive data at the same time, or can I do this with 1 socket.
thanks,
-neko
Heres another question...the architecture we just spoke about outlines how the server would operate...how would the client be setup? I''m assuming itd be just creating one socket, connecting to the server, and then reading/writing off that socket. But are sockets bidirectional and asynchronous? (for example can I read off of a socket and write off of that same socket in parallel, or is it only able to transport data in one direction at a time?) I suppose what Im getting at is do I need 2 sockets for client/server communication if I want to be able to send data and receive data at the same time, or can I do this with 1 socket.
thanks,
-neko
Sockets are, as you say, "bidirectional".
However, using two threads to read and write on the same socket at the same time (or write and select at the same time) may cause trouble, depending on your OS version and phase of the moon. I wouldn''t recommend it.
However, using two threads to read and write on the same socket at the same time (or write and select at the same time) may cause trouble, depending on your OS version and phase of the moon. I wouldn''t recommend it.
Author
I can only foresee a client having one thread and I’m now implementing the server architecture with a single thread by calling the select function. If I had know about the select function beforehand I definitely wouldn’t have proposed such an implementation.
Thanks,
-Aphex
Thanks,
-Aphex
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
