4 files changed, 218 insertions, 33 deletions

diff --git a/source/cClientHandle.cpp b/source/cClientHandle.cpp
index 78032611e..9eed31a3a 100644
--- a/source/cClientHandle.cpp
+++ b/source/cClientHandle.cpp
@@ -151,7 +151,7 @@ cClientHandle::~cClientHandle()
 {

 	LOG("Deleting client \"%s\"", GetUsername().c_str());

 

-	// Remove from cSocketThreads, just in case

+	// Remove from cSocketThreads, we're not to be called anymore:

 	cRoot::Get()->GetServer()->ClientDestroying(this);

 	

 	m_LoadedChunks.clear();

@@ -225,14 +225,6 @@ void cClientHandle::Destroy()
 	{

 		RemoveFromAllChunks();

 	}

-	

-	if (m_Socket.IsValid())

-	{

-		m_Socket.CloseSocket();

-	}

-	

-	// Synchronize with the cSocketThreads (so that they don't call us anymore)

-	cRoot::Get()->GetServer()->ClientDestroying(this);

 }

 

 

@@ -489,6 +481,7 @@ void cClientHandle::HandlePacket(cPacket * a_Packet)
 				

 				// Ignored packets:

 				case E_PLAYERLOOK:

+				case E_CHAT:

 				case E_PLAYERMOVELOOK:

 				case E_PLAYERPOS:

 				case E_KEEP_ALIVE:     break;

@@ -504,6 +497,8 @@ void cClientHandle::HandlePacket(cPacket * a_Packet)
 			{

 				// Ignored packets:

 				case E_KEEP_ALIVE:

+				case E_CHAT:

+				case E_FLYING:

 				case E_PLAYERLOOK:

 				case E_PLAYERMOVELOOK:

 				case E_PLAYERPOS: break;

@@ -520,6 +515,8 @@ void cClientHandle::HandlePacket(cPacket * a_Packet)
 			{

 				// Ignored packets:

 				case E_KEEP_ALIVE:

+				case E_CHAT:

+				case E_FLYING:

 				case E_PLAYERLOOK:

 				case E_PLAYERMOVELOOK:

 				case E_PLAYERPOS: break;

@@ -535,6 +532,8 @@ void cClientHandle::HandlePacket(cPacket * a_Packet)
 			{

 				// Ignored packets:

 				case E_KEEP_ALIVE:

+				case E_CHAT:

+				case E_FLYING:

 				case E_PLAYERLOOK:

 				case E_PLAYERPOS: break;

 

diff --git a/source/cServer.cpp b/source/cServer.cpp
index 8462fa915..97bcffaae 100644
--- a/source/cServer.cpp
+++ b/source/cServer.cpp
@@ -102,7 +102,7 @@ void cServer::ServerListenThread( void *a_Args )
 

 void cServer::ClientDestroying(const cClientHandle * a_Client)

 {

-	m_SocketThreads.RemoveClient(a_Client);

+	m_SocketThreads.StopReading(a_Client);

 }

 

 

diff --git a/source/cSocketThreads.cpp b/source/cSocketThreads.cpp
index aa0aba678..5e98398c4 100644
--- a/source/cSocketThreads.cpp
+++ b/source/cSocketThreads.cpp
@@ -85,7 +85,9 @@ void cSocketThreads::RemoveClient(const cSocket * a_Socket)
 		{

 			return;

 		}

-	}

+	}  // for itr - m_Threads[]

+	

+	ASSERT(!"Removing an unknown socket");

 }

 

 

@@ -103,7 +105,9 @@ void cSocketThreads::RemoveClient(const cCallback * a_Client)
 		{

 			return;

 		}

-	}

+	}  // for itr - m_Threads[]

+

+	ASSERT(!"Removing an unknown client");

 }

 

 

@@ -121,7 +125,81 @@ void cSocketThreads::NotifyWrite(const cCallback * a_Client)
 		{

 			return;

 		}

+	}  // for itr - m_Threads[]

+	

+	// Cannot assert - this normally happens if a client disconnects and has pending packets, the cServer::cNotifyWriteThread will call this on invalid clients too

+	// ASSERT(!"Notifying write to an unknown client");

+}

+

+

+

+

+

+void cSocketThreads::Write(const cSocket * a_Socket, const AString & a_Data)

+{

+	// Puts a_Data into outgoing data queue for a_Socket

+	

+	if (!a_Socket->IsValid())

+	{

+		// Socket already closed, ignore the request

+		return;

+	}

+	

+	cCSLock Lock(m_CS);

+	for (cSocketThreadList::iterator itr = m_Threads.begin(); itr != m_Threads.end(); ++itr)

+	{

+		if ((*itr)->Write(a_Socket, a_Data))

+		{

+			return;

+		}

+	}  // for itr - m_Threads[]

+	

+	ASSERT(!"Writing to an unknown socket");

+}

+

+

+

+

+

+/// Stops reading from the socket - when this call returns, no more calls to the callbacks are made

+void cSocketThreads::StopReading(const cCallback * a_Client)

+{

+	cCSLock Lock(m_CS);

+	for (cSocketThreadList::iterator itr = m_Threads.begin(); itr != m_Threads.end(); ++itr)

+	{

+		if ((*itr)->StopReading(a_Client))

+		{

+			return;

+		}

+	}  // for itr - m_Threads[]

+	

+	// Cannot assert, this normally happens if the socket is closed before the client deinitializes

+	// ASSERT(!"Stopping reading on an unknown client");

+}

+

+

+

+

+

+/// Queues the socket for closing, as soon as its outgoing data is sent

+void cSocketThreads::QueueClose(const cSocket * a_Socket)

+{

+	if (!a_Socket->IsValid())

+	{

+		// Already closed, ignore the request

+		return;

 	}

+	

+	cCSLock Lock(m_CS);

+	for (cSocketThreadList::iterator itr = m_Threads.begin(); itr != m_Threads.end(); ++itr)

+	{

+		if ((*itr)->QueueClose(a_Socket))

+		{

+			return;

+		}

+	}  // for itr - m_Threads[]

+	

+	ASSERT(!"Queueing close of an unknown socket");

 }

 

 

@@ -189,8 +267,7 @@ bool cSocketThreads::cSocketThread::RemoveClient(const cCallback * a_Client)
 		}

 		

 		// Found, remove it:

-		m_Slots[i] = m_Slots[m_NumSlots - 1];

-		m_NumSlots--;

+		m_Slots[i] = m_Slots[--m_NumSlots];

 		

 		// Notify the thread of the change:

 		ASSERT(m_ControlSocket2.IsValid());

@@ -210,11 +287,6 @@ bool cSocketThreads::cSocketThread::RemoveSocket(const cSocket * a_Socket)
 {

 	// Returns true if removed, false if not found

 

-	if (m_NumSlots == 0)

-	{

-		return false;

-	}

-	

 	for (int i = m_NumSlots - 1; i >= 0 ; --i)

 	{

 		if (m_Slots[i].m_Socket != a_Socket)

@@ -223,8 +295,7 @@ bool cSocketThreads::cSocketThread::RemoveSocket(const cSocket * a_Socket)
 		}

 		

 		// Found, remove it:

-		m_Slots[i] = m_Slots[m_NumSlots - 1];

-		m_NumSlots--;

+		m_Slots[i] = m_Slots[--m_NumSlots];

 		

 		// Notify the thread of the change:

 		ASSERT(m_ControlSocket2.IsValid());

@@ -240,6 +311,38 @@ bool cSocketThreads::cSocketThread::RemoveSocket(const cSocket * a_Socket)
 

 

 

+bool cSocketThreads::cSocketThread::HasClient(const cCallback * a_Client) const

+{

+	for (int i = m_NumSlots - 1; i >= 0; --i)

+	{

+		if (m_Slots[i].m_Client == a_Client)

+		{

+			return true;

+		}

+	}  // for i - m_Slots[]

+	return false;

+}

+

+

+

+

+

+bool cSocketThreads::cSocketThread::HasSocket(const cSocket * a_Socket) const

+{

+	for (int i = m_NumSlots - 1; i >= 0; --i)

+	{

+		if (m_Slots[i].m_Socket->GetSocket() == a_Socket->GetSocket())

+		{

+			return true;

+		}

+	}  // for i - m_Slots[]

+	return false;

+}

+

+

+

+

+

 bool cSocketThreads::cSocketThread::NotifyWrite(const cCallback * a_Client)

 {

 	if (HasClient(a_Client))

@@ -256,12 +359,43 @@ bool cSocketThreads::cSocketThread::NotifyWrite(const cCallback * a_Client)
 

 

 

-bool cSocketThreads::cSocketThread::HasClient(const cCallback * a_Client) const

+bool cSocketThreads::cSocketThread::Write(const cSocket * a_Socket, const AString & a_Data)

 {

+	// Returns true if socket handled by this thread

+	for (int i = m_NumSlots - 1; i >= 0; --i)

+	{

+		if (m_Slots[i].m_Socket == a_Socket)

+		{

+			m_Slots[i].m_Outgoing.append(a_Data);

+			

+			// Notify the thread that there's data in the queue:

+			ASSERT(m_ControlSocket2.IsValid());

+			m_ControlSocket2.Send("q", 1);

+			

+			return true;

+		}

+	}  // for i - m_Slots[]

+	return false;

+}

+

+

+

+

+

+bool cSocketThreads::cSocketThread::StopReading (const cCallback * a_Client)

+{

+	// Returns true if client handled by this thread

 	for (int i = m_NumSlots - 1; i >= 0; --i)

 	{

 		if (m_Slots[i].m_Client == a_Client)

 		{

+			m_Slots[i].m_Client = NULL;

+			m_Slots[i].m_ShouldClose = false;

+

+			// Notify the thread that there's a stop reading request:

+			ASSERT(m_ControlSocket2.IsValid());

+			m_ControlSocket2.Send("s", 1);

+			

 			return true;

 		}

 	}  // for i - m_Slots[]

@@ -272,12 +406,20 @@ bool cSocketThreads::cSocketThread::HasClient(const cCallback * a_Client) const
 

 

 

-bool cSocketThreads::cSocketThread::HasSocket(const cSocket * a_Socket) const

+bool cSocketThreads::cSocketThread::QueueClose(const cSocket * a_Socket)

 {

+	// Returns true if socket handled by this thread

 	for (int i = m_NumSlots - 1; i >= 0; --i)

 	{

-		if (m_Slots[i].m_Socket->GetSocket() == a_Socket->GetSocket())

+		if (m_Slots[i].m_Socket == a_Socket)

 		{

+			ASSERT(m_Slots[i].m_Client == NULL);  // Should have stopped reading first

+			m_Slots[i].m_ShouldClose = true;

+

+			// Notify the thread that there's a close queued (in case its conditions are already met):

+			ASSERT(m_ControlSocket2.IsValid());

+			m_ControlSocket2.Send("c", 1);

+			

 			return true;

 		}

 	}  // for i - m_Slots[]

@@ -453,17 +595,26 @@ void cSocketThreads::cSocketThread::ReadFromSockets(fd_set * a_Read)
 		{

 			// The socket has been closed by the remote party, close our socket and let it be removed after we process all reading

 			m_Slots[i].m_Socket->CloseSocket();

-			m_Slots[i].m_Client->SocketClosed();

+			if (m_Slots[i].m_Client != NULL)

+			{

+				m_Slots[i].m_Client->SocketClosed();

+			}

 		}

 		else if (Received > 0)

 		{

-			m_Slots[i].m_Client->DataReceived(Buffer, Received);

+			if (m_Slots[i].m_Client != NULL)

+			{

+				m_Slots[i].m_Client->DataReceived(Buffer, Received);

+			}

 		}

 		else

 		{

 			// The socket has encountered an error, close it and let it be removed after we process all reading

 			m_Slots[i].m_Socket->CloseSocket();

-			m_Slots[i].m_Client->SocketClosed();

+			if (m_Slots[i].m_Client != NULL)

+			{

+				m_Slots[i].m_Client->SocketClosed();

+			}

 		}

 	}  // for i - m_Slots[]

 }

@@ -485,10 +636,19 @@ void cSocketThreads::cSocketThread::WriteToSockets(fd_set * a_Write)
 		if (m_Slots[i].m_Outgoing.empty())

 		{

 			// Request another chunk of outgoing data:

-			m_Slots[i].m_Client->GetOutgoingData(m_Slots[i].m_Outgoing);

+			if (m_Slots[i].m_Client != NULL)

+			{

+				m_Slots[i].m_Client->GetOutgoingData(m_Slots[i].m_Outgoing);

+			}

 			if (m_Slots[i].m_Outgoing.empty())

 			{

-				// Nothing ready yet

+				// Nothing ready

+				if ((m_Slots[i].m_Client == NULL) && m_Slots[i].m_ShouldClose)

+				{

+					// Socket was queued for closing and there's no more data to send, close it now:

+					m_Slots[i].m_Socket->CloseSocket();

+					m_Slots[i] = m_Slots[--m_NumSlots];

+				}

 				continue;

 			}

 		}  // if (outgoing data is empty)

@@ -496,9 +656,13 @@ void cSocketThreads::cSocketThread::WriteToSockets(fd_set * a_Write)
 		int Sent = m_Slots[i].m_Socket->Send(m_Slots[i].m_Outgoing.data(), m_Slots[i].m_Outgoing.size());

 		if (Sent < 0)

 		{

-			LOGWARNING("Error while writing to client \"%s\", disconnecting", m_Slots[i].m_Socket->GetIPString().c_str());

+			int Err = cSocket::GetLastError();

+			LOGWARNING("Error %d while writing to client \"%s\", disconnecting. \"%s\"", Err, m_Slots[i].m_Socket->GetIPString().c_str(), cSocket::GetErrorString(Err).c_str());

 			m_Slots[i].m_Socket->CloseSocket();

-			m_Slots[i].m_Client->SocketClosed();

+			if (m_Slots[i].m_Client != NULL)

+			{

+				m_Slots[i].m_Client->SocketClosed();

+			}

 			return;

 		}

 		m_Slots[i].m_Outgoing.erase(0, Sent);

@@ -531,8 +695,7 @@ void cSocketThreads::cSocketThread::RemoveClosedSockets(void)
 		{

 			continue;

 		}

-		m_Slots[i] = m_Slots[m_NumSlots - 1];

-		m_NumSlots--;

+		m_Slots[i] = m_Slots[--m_NumSlots];

 	}  // for i - m_Slots[]

 }

 

diff --git a/source/cSocketThreads.h b/source/cSocketThreads.h
index a32808aeb..a8174a115 100644
--- a/source/cSocketThreads.h
+++ b/source/cSocketThreads.h
@@ -5,6 +5,16 @@
 // This object takes care of network communication, groups sockets into threads and uses as little threads as possible for full read / write support

 // For more detail, see http://forum.mc-server.org/showthread.php?tid=327

 

+/*

+Additional details:

+When a client is terminating a connection:

+- they call the StopReading() method to disable callbacks for the incoming data

+- they call the Write() method to queue any outstanding outgoing data

+- they call the QueueClose() method to queue the socket to close after outgoing data has been sent.

+When a socket slot is marked as having no callback, it is kept alive until its outgoing data queue is empty and its m_ShouldClose flag is set.

+This means that the socket can be written to several times before finally closing it via QueueClose()

+*/

+

 

 

 

@@ -76,6 +86,15 @@ public:
 	

 	/// Notify the thread responsible for a_Client that the client has something to write

 	void NotifyWrite(const cCallback * a_Client);

+	

+	/// Puts a_Data into outgoing data queue for a_Socket

+	void Write(const cSocket * a_Socket, const AString & a_Data);

+	

+	/// Stops reading from the socket - when this call returns, no more calls to the callbacks are made

+	void StopReading(const cCallback * a_Client);

+	

+	/// Queues the socket for closing, as soon as its outgoing data is sent

+	void QueueClose(const cSocket * a_Socket);

 

 private:

 

@@ -99,6 +118,9 @@ private:
 		bool HasClient   (const cCallback * a_Client) const;

 		bool HasSocket   (const cSocket *   a_Socket) const;

 		bool NotifyWrite (const cCallback * a_Client);  // Returns true if client handled by this thread

+		bool Write       (const cSocket *   a_Socket, const AString & a_Data);  // Returns true if socket handled by this thread

+		bool StopReading (const cCallback * a_Client);  // Returns true if client handled by this thread

+		bool QueueClose  (const cSocket *   a_Socket);  // Returns true if socket handled by this thread

 		

 		bool Start(void);  // Hide the cIsThread's Start method, we need to provide our own startup to create the control socket

 		

@@ -119,6 +141,7 @@ private:
 			cSocket *   m_Socket;

 			cCallback * m_Client;

 			AString     m_Outgoing;  // If sending writes only partial data, the rest is stored here for another send

+			bool        m_ShouldClose;  // If true, the socket is to be closed after sending all outgoing data

 		} ;

 		sSlot m_Slots[MAX_SLOTS];

 		int   m_NumSlots;  // Number of slots actually used