Sending list of connected users to newly connected user in multithreaded iocp server

Question

I need some advice how to send properly doubly linked list of connected users. Some basic information about my code and my approach so far:

I keep information about all connected users in doubly linked list, which is shared among threads. I store the head of the list in global variable : *PPER_user g_usersList, and struct for users look like:

typedef struct _user {
  char      id;
  char      status;
  struct _user  *pCtxtBack; 
  struct _user  *pCtxtForward;
} user, *PPER_user;

When new user connects to server, data about connected users is gathered from linked list and send to him:

 WSABUF wsabuf; PPER_player pTemp1, pTemp2; unsigned int c=0;
 .....
 EnterCriticalSection(&g_CSuserslist); 
 pTemp1 = g_usersList; 
 while( pTemp1 ) {
   pTemp2 = pTemp1->pCtxtBack;
   wsabuf.buf[c++]=pTemp1->id;     // fill buffer with data about all users
   wsabuf.buf[c++]=pTemp1->status; //
   pTemp1 = pTemp2;
   };
 WSASend(...,wsabuf,...);
 LeaveCriticalSection(&g_CSuserslist);

But a few things about code above makes me confused:

1. the linked list is rather under heavily usage by other threads. The more connected users (for example 100,1000), the longer period of time list is locked for the entire duration of ghatering data. Should I reconcile with that or find some better way to do this?

2. it seems that when one thread locks list whilst while loop went trough all chained struct(users) gathering all id, status , other threads should use the same CriticalSection(&g_CSuserslist) when users want to change their own id, status etc. But this will likely kill the performance. Maybe should i change all design of my app or something?

Any insight you may have would be appreciated. Thanks in advance.

Answer 1

The only problem I see in your code (and more generally in the description of your app) is the size of the critical section that protects g_usersList. The rule is avoid any time consuming operation while in critical section.

So you must protect :

• adding a new user
• removing a user at deconnexion
• taking a snapshot of the list for further processing

All those operation are memory only, so unless you go under really heavy conditions, all should be fine provided you put all IO outside of critical sections ⁽¹⁾, because it only happens when users are connecting/disconnecting. If you put the WSASend outside of critical section, all should go fine and IMHO it is enough.

Edit per comment :

Your struct user is reasonably small, I would say between 10 and 18 useful bytes (depending on pointer size 4 or 8 bytes), and a total of 12 of 24 bytes including padding. With 1000 connected users you only have to copy less then 24k bytes of memory and having only to test if next user is null (or at most keep the current number of connected user to have a simpler loop). Anyway, maintaining such a buffer should also be done in a critical section. IMHO until you have far more than 1000 users (between 10k and 100k, but you could get other problems ...) a simple global lock (like your critical section) around the whole double linked list of user should be enough. But all that needs to be probed because it may depend of external things like hardware ...

---

Too Long Don't Read discussion :

As you describe your application, you only gather the list of connected users when a new users connects, so you have exactly one full read per two writes (one at connection and one at deconnection) : IMHO it is no use trying to implement share locks for reading and exclusive ones for writing. If you did many reads between a connection and a deconnection, it won't be same thing, and you should try to allow concurrent reads.

If you really find the contention is too heavy, because you have a very large number of connected users and very frequent connection/disconnection, you could try to implement a row level like locking. Instead of locking the whole list, only lock what you are processing : top and first for an insertion, current record plus previous and next for a deletion, and current and next while reading. But it will be hard to write and test, much more time consuming, because you will have to do many lock/release while reading the list, and you will have to be very cautious to avoid dead lock condition. So my advice is don't do that unless it is really required.

---

(1) in the code you show, the WSASend(...,wsabuf,...); is inside the critical section when it should be outside. Write instead :

...
LeaveCriticalSection(&g_CSuserslist);
WSASend(...,wsabuf,...);

Answer 2

The first performance problem is the linked list itself: It takes quite a bit longer to traverse a linked list than to traverse an array/std::vector<>. A single linked list has the advantage of allowing thread safe insertion/deletion of elements via atomic types/compare-and-swap operations. A double linked list is much harder to maintain in a thread safe fashion without resorting to mutexes (which are always the big, heavy guns).

So, if you go with mutex to lock the list, use std::vector<>, but you can also solve your problem with a lock-free implementation of a single linked list:

• You have a single linked list with one head that is a global, atomic variable.

• All entries are immutable once they are published.

• When you add a user, take the current head and store it in a thread local variable (atomic read). Since the entries won't change, you have all time in the world to traverse this list, even if other threads add more users while you are traversing it.

• To add the new user, create a new list head containing it, then use a compare-and-swap operation to replace the old list head pointer with the new one. If that fails, retry.

• To remove a user, traverse the list until you find the user in the list. While you walk the list, copy its contents to newly allocated nodes in a new linked list. Once you find the user to delete, set the next pointer of the last user on the new list to the deleted user's next pointer. Now the new list contains all users of the old one except the removed user. So you can now publish that list by another compare-and-swap on the list head. Unfortunately, you'll have to redo the work should the publishing operation fail.

  Do not set the next pointer of the deleted object to NULL, another thread might still need it to find the rest of the list (in its view the object won't have been removed yet).

  Do not delete the old list head right away, another thread might still be using it. The best thing to do is to enqueue its nodes in another list for cleanup. This cleanup list should be replaced from time to time with a new one, and the old one should be cleaned up after all threads have given their OK (you can implement this by passing around a token, when it comes back to the originating process, you can safely destroy the old objects.

Since the list head pointer is the only globally visible variable that can ever change, and since that variable is atomic, such an implementation guarantees a total ordering of all add/remove operations.

Answer 3

The "correct" answer is probably to send less data to your users. Do they really NEED to know the id and status of every other user or do they only need to know aggregate information which can be kept up-to-date dynamically.

If your app must send this information (or such changes are considered too much work), then you could cut down your processing significantly by only making this calculation, say, once per second (or even per minute). Then, when someone logged on, they would receive a copy of this information that is, at most, 1 second old.

Answer 4

The real question here is just how urgent it is to send every byte of that list to the new user?

How well does the client side track this list data?

If the client can handle partial updates, wouldn't it make more sense to 'trickle' the data to each user - perhaps using a timestamp to indicate freshness of the data and not have to lock the list in such a massive fashion?

You could also switch to a rwsem style lock where list access is only exclusive if the user intends to modify the list.