Hi.

I sort of have a server running but when there is 2 or more clients the sever crashes with a Debug Assertion Fail

in xstring line 78 what does that mean with in the following code.

the server app will send a predefind vector of messages to all clients when a button is pressed

Like so.

for(UINT ctr = 0; ctr < Messages.size(); ++ctr)

Lobby.deliver(Messages[ctr]);

if there is only one client it seams to work.

If I just send 1 message it all works can hold the send button down and it sends data no fails

Lobby.deliver(Messages[0]);

It must happen afer the data is sent because the 2 clients recieve the correct data.

where should I look.

I'm thinking it may be a race condition with the message and the async_write method.

both handle_write may return at the same time.

The Lobby Deliver Function

.

 void deliver(const cMessage& msg)
  {
//first time user of std::for_each
    std::for_each(Clients.begin(), Clients.end(),
        boost::bind(&cConnection::deliver, _1, boost::ref(msg)));
  }

and the cConnections deliver that gets binded

.

 void deliver(const cMessage& msg)
  {
   //the sever sends use messages by this
      async_write(msg, boost::bind(&cSession::handle_write, this,
				boost::asio::placeholders::error, shared_from_this()));
  }
  


  /// Handle completion of a write operation.
  void handle_write(const boost::system::error_code& e, cConnection_ptr client)
  {
    // Nothing to do. The socket will be closed automatically when the last
    // reference to the connection object goes away.


	std::stringstream ss;
	ss << "Server Handle Write Completed \n";

	global_stream_lock.lock();
	//send the data to the client list box
	SendMessage(GlobalList, (UINT)LB_ADDSTRING , 0, (LPARAM) ss.str().c_str()); 
	global_stream_lock.unlock();


		if(!e)// && read_msg_.decode_header())
		{
			//std::cout << "Server Sent All DATA" << std::endl;
			ss.str("");
			ss.clear();
			ss << "Server Sent All DATA";

	global_stream_lock.lock();
	//send the data to the client list box
	SendMessage(GlobalList, (UINT)LB_ADDSTRING , 0, (LPARAM) ss.str().c_str()); 
	global_stream_lock.unlock();


		}
		else
		{
			//std::cout << "Server Send Error = " << e.message << std::endl;
			ss.str("");
			ss.clear();
			ss << "Server Send Error = " << e.message();

	global_stream_lock.lock();
	//send the data to the client list box
	SendMessage(GlobalList, (UINT)LB_ADDSTRING , 0, (LPARAM) ss.str().c_str()); 
	global_stream_lock.unlock();

	ss.str("");
			ss.clear();
			ss << "Lost Client\nRemoving Client = FooBar";

	global_stream_lock.lock();
	//send the data to the client list box
	SendMessage(GlobalList, (UINT)LB_ADDSTRING , 0, (LPARAM) ss.str().c_str()); 
	global_stream_lock.unlock();


//will the lobby need to be protected by mute x lock or some other ????????????
			//remove this client from the lobby its a dead client
			Lobby.leave(shared_from_this());

		}
  }

where should I look.

Umm... maybe with the assert? What condition caused the assert to fail? What is the stack trace? What are the values in the function calls leading up to the assert? Are they what you expected?

No one here is going to be able to help you based on what you've provided. You're the one with the repro and the debugger at your fingertips.

Hi.

All I have was that dlg box with the debug assert., no statck trace nothing.

it works til some time after the data gets sent. which points to the call back after a async write is completed. I'll place a break point in there to see.

the message is valid.

What about async writes in a loop it needs the message until it sends all the bits, hows that loop affect the message. It keeps a referance to it.

but I dont free the buffer until app shut down.

Message is a vector<cMessage> Message;

The only thing in the call back for write completed is info to the main window

global_stream_lock.lock();

//send the data to the client list box

SendMessage(GlobalList, (UINT)LB_ADDSTRING , 0, (LPARAM) ss.str().c_str());

global_stream_lock.unlock();

its a win32 listbox, what could have some thing to do with the async call backs posting to the list box at the same time may be I'll strip that function clean and run it after tea.

I removed all the SendMessage win32 functions and it worked.

found a new error. when a clien is disconnected the handle_write complete call back was removing clients, but if I sent 10 messages to that client this would remove the client

before the other 9 messages completed and detected lost client and try to remove it again. What should I do in that case, maybe stor the client in a remove list and remove them later.

I delete the clien from a std::set like so

void leave(cConnection_ptr client)

{

Clients.erase(client);

}

You're using threading, are you sure the object's lifetime persists until the callback executes? How are you ensuring your "Clients" collection is not being accessed by two threads at once?

Some observations:
You should not mutate a container while you're iterating over it (or otherwise have an iterator alive that references it.)
Building a separate set of "clients to remove" is a good idea.
Separating UI from data from networking would be a great idea! Make the UI generate commands that go into a queue, rather than try to mutate data directly.
You likely don't need threads. Remove them until it's proven that you need them.

Hi all, and thanks.

Ok it was a race condition removing clients I now place a mutex around the Lobby, Leave and deliver and join functions.

Now I can safely remove all clients.

Ok so I have a start I'm going to clean the classes up and post them to see if there is any really bad habits or things I have not handled to well.

I will also need advice on holding and passing app data to and from network code. But wait till I post the classes, I'll make a new post for that.

As hplus0603 mentioned, threading might be unnecessary. Multi-threading with locking can perform similar or worse than single threaded code. When you do need threading, it is often better to avoid explicit locking and instead make a thread safe communication mechanism, e.g. a thread safe queue, and implement the design in terms of a classic thread model, e.g. producer / consumer.

Having locking scattered "randomly" throughout the rest of the code is a recipe for complexity and bugs, and can make it hard to reason about issues like deadlocks.

As hplus0603 mentioned, threading might be unnecessary. Multi-threading with locking can perform similar or worse than single threaded code. When you do need threading, it is often better to avoid explicit locking and instead make a thread safe communication mechanism, e.g. a thread safe queue, and implement the design in terms of a classic thread model, e.g. producer / consumer.

Having locking scattered "randomly" throughout the rest of the code is a recipe for complexity and bugs, and can make it hard to reason about issues like deadlocks.

I agree with you. I don't like the locks there, This is more about learning to use threads correctly for me its now time to use them since the cpus are now made for it.

Im not that green with using threads but only used locks before. I'll start looking into thread safe communication mechanism. That code I posted was just to get up and running, So I could get a feel of how boost::asio works, I could readf and read allday about networking but to understand you need to get your hands dirty, and that mean make lots of errors no errors == no learning.

Don't learn threads at the same time you're learning multiplayer at the same time you're learning C++ container classes and standard library.
Do one at a time!