Fixed a really nasty receive bug (TCP only). Using SCTP would be so much better...

src/net/clientcontext.h

@@ -22,7 +22,7 @@
 #define _CLIENTCONTEXT_H_
 
 #include <net/netcontext.h>
-
+#include <net/receivebuffer.h>
 
 class ClientContext : public NetContext
 {
@@ -67,6 +67,9 @@ public:
 	int GetClientSockaddrSize() const
 	{return m_addrFamily == AF_INET6 ? sizeof(sockaddr_in6) : sizeof(sockaddr_in);}
 
+	ReceiveBuffer &GetReceiveBuffer()
+	{return m_receiveBuffer;}
+
 private:
 	SOCKET				m_sockfd;
 	int					m_protocol;
@@ -76,6 +79,7 @@ private:
 	std::string			m_password;
 	sockaddr_storage	m_clientSockaddr;
 	std::string			m_playerName;
+	ReceiveBuffer		m_receiveBuffer;
 };
 
 #endif

src/net/common/clientstate.cpp

@@ -376,7 +376,8 @@ AbstractClientStateReceiving::Process(ClientThread &client)
 	int retVal = MSG_SOCK_INTERNAL_PENDING;
 
 	// delegate to receiver helper class
-	boost::shared_ptr<NetPacket> tmpPacket = client.GetReceiver().Recv(client.GetContext().GetSocket());
+	boost::shared_ptr<NetPacket> tmpPacket =
+		client.GetReceiver().Recv(client.GetContext().GetSocket(), client.GetContext().GetReceiveBuffer());
 
 	if (tmpPacket.get())
 	{

src/net/common/receiverhelper.cpp

@@ -26,7 +26,6 @@ using namespace std;
 
 
 ReceiverHelper::ReceiverHelper()
-: m_socket(INVALID_SOCKET), m_tmpInBufSize(0)
 {
 }
 
@@ -34,25 +33,14 @@ ReceiverHelper::~ReceiverHelper()
 {
 }
 
-void
-ReceiverHelper::Init(SOCKET socket)
-{
-	if (!IS_VALID_SOCKET(socket))
-		return; // TODO: throw exception
-
-	m_socket = socket;
-}
-
 boost::shared_ptr<NetPacket>
-ReceiverHelper::Recv(SOCKET sock)
+ReceiverHelper::Recv(SOCKET sock, ReceiveBuffer &buf)
 {
-	boost::shared_ptr<NetPacket> tmpPacket(InternalGetPacket());
+	if (buf.receivedPackets.empty())
-
-	if (!tmpPacket.get())
 	{
-		unsigned bufSize = RECV_BUF_SIZE - m_tmpInBufSize;
+		int bufSize = RECV_BUF_SIZE - buf.recvBufUsed;
 
-		if (bufSize) // check if there is room in the input buffer
+		if (bufSize > 0) // check if there is room in the input buffer
 		{
 			fd_set readSet;
 			struct timeval timeout;
@@ -69,7 +57,7 @@ ReceiverHelper::Recv(SOCKET sock)
 			}
 			if (selectResult > 0) // recv is possible
 			{
-				int bytesRecvd = recv(sock, m_tmpInBuf + m_tmpInBufSize, bufSize, 0);
+				int bytesRecvd = recv(sock, buf.recvBuf + buf.recvBufUsed, bufSize, 0);
 
 				if (!IS_VALID_RECV(bytesRecvd))
 				{
@@ -81,37 +69,49 @@ ReceiverHelper::Recv(SOCKET sock)
 				}
 				else
 				{
-					m_tmpInBufSize += bytesRecvd;
+					buf.recvBufUsed += bytesRecvd;
-					tmpPacket = InternalGetPacket();
+					InternalGetPackets(buf);
 				}
 			}
 		}
 	}
-	return tmpPacket;
-}
-
-boost::shared_ptr<NetPacket>
-ReceiverHelper::InternalGetPacket()
-{
 	boost::shared_ptr<NetPacket> tmpPacket;
-
+	if (!buf.receivedPackets.empty())
-	// This is necessary, because we use TCP.
-	// Packets may be received in multiple chunks or
-	// several packets may be received at once.
-	if (m_tmpInBufSize >= MIN_PACKET_SIZE)
 	{
-		try
+		tmpPacket = buf.receivedPackets.front();
-		{
+		buf.receivedPackets.pop_front();
-			// This call will also handle the memmove stuff, i.e.
-			// buffering for partial packets.
-			tmpPacket = NetPacket::Create(m_tmpInBuf, m_tmpInBufSize);
-		} catch (const NetException &)
-		{
-			// Reset buffer on error.
-			m_tmpInBufSize = 0;
-			// TODO: log error/increase error counter.
-		}
 	}
 	return tmpPacket;
 }
 
+void
+ReceiverHelper::InternalGetPackets(ReceiveBuffer &buf)
+{
+	bool dataAvailable = true;
+	do
+	{
+		boost::shared_ptr<NetPacket> tmpPacket;
+		// This is necessary, because we use TCP.
+		// Packets may be received in multiple chunks or
+		// several packets may be received at once.
+		if (buf.recvBufUsed >= MIN_PACKET_SIZE)
+		{
+			try
+			{
+				// This call will also handle the memmove stuff, i.e.
+				// buffering for partial packets.
+				tmpPacket = NetPacket::Create(buf.recvBuf, buf.recvBufUsed);
+			} catch (const NetException &)
+			{
+				// Reset buffer on error.
+				buf.recvBufUsed = 0;
+				// TODO: log error/increase error counter.
+			}
+		}
+		if (tmpPacket.get())
+			buf.receivedPackets.push_back(tmpPacket);
+		else
+			dataAvailable = false;
+	} while(dataAvailable);
+}
+

src/net/common/servergamestate.cpp

@@ -139,7 +139,7 @@ AbstractServerGameStateReceiving::Process(ServerGameThread &server)
 		try
 		{
 			// Receive the packet.
-			packet = server.GetReceiver().Recv(session.sessionData->GetSocket());
+			packet = server.GetReceiver().Recv(session.sessionData->GetSocket(), session.sessionData->GetReceiveBuffer());
 		} catch (const NetException &)
 		{
 			server.CloseSessionDelayed(session);

src/net/common/serverlobbythread.cpp

@@ -193,7 +193,7 @@ ServerLobbyThread::ProcessLoop()
 		try
 		{
 			// Receive the next packet.
-			packet = GetReceiver().Recv(session.sessionData->GetSocket());
+			packet = GetReceiver().Recv(session.sessionData->GetSocket(), session.sessionData->GetReceiveBuffer());
 		} catch (const NetException &)
 		{
 			// On error: Close this session.

src/net/common/sessionmanager.cpp

@@ -95,44 +95,55 @@ SessionManager::Select(unsigned timeoutMsec)
 
 		while (i != end)
 		{
+			// Collect all sockets.
 			SOCKET tmpSock = i->first;
 			FD_SET(tmpSock, &rdset);
 			if (tmpSock > maxSock || maxSock == INVALID_SOCKET)
 				maxSock = tmpSock;
+
+			// Check if a packet is available.
+			if (!i->second.sessionData->GetReceiveBuffer().receivedPackets.empty())
+			{
+				retSession = i->second;
+				break;
+			}
 			++i;
 		}
 	}
 
-	if (maxSock == INVALID_SOCKET)
+	if (!retSession.sessionData.get())
 	{
-		Thread::Msleep(timeoutMsec); // just sleep if there is no session
+		if (maxSock == INVALID_SOCKET)
-	}
-	else
-	{
-		// wait for data
-		struct timeval timeout;
-		timeout.tv_sec = timeoutMsec / 1000;
-		timeout.tv_usec = (timeoutMsec % 1000) * 1000;
-		int selectResult = select(maxSock + 1, &rdset, NULL, NULL, &timeout);
-		if (!IS_VALID_SELECT(selectResult))
 		{
-			throw ServerException(ERR_SOCK_SELECT_FAILED, SOCKET_ERRNO());
+			Thread::Msleep(timeoutMsec); // just sleep if there is no session
 		}
-		if (selectResult > 0) // one (or more) of the sockets is readable
+		else
 		{
-			// Check which socket is readable, return the first.
+			// wait for data
-			boost::mutex::scoped_lock lock(m_sessionMapMutex);
+			struct timeval timeout;
-			SessionMap::iterator i = m_sessionMap.begin();
+			timeout.tv_sec = timeoutMsec / 1000;
-			SessionMap::iterator end = m_sessionMap.end();
+			timeout.tv_usec = (timeoutMsec % 1000) * 1000;
-
+			int selectResult = select(maxSock + 1, &rdset, NULL, NULL, &timeout);
-			while (i != end)
+			if (!IS_VALID_SELECT(selectResult))
 			{
-				if (FD_ISSET(i->first, &rdset))
+				throw ServerException(ERR_SOCK_SELECT_FAILED, SOCKET_ERRNO());
+			}
+			if (selectResult > 0) // one (or more) of the sockets is readable
+			{
+				// Check which socket is readable, return the first.
+				boost::mutex::scoped_lock lock(m_sessionMapMutex);
+				SessionMap::iterator i = m_sessionMap.begin();
+				SessionMap::iterator end = m_sessionMap.end();
+
+				while (i != end)
 				{
-					retSession = i->second;
+					if (FD_ISSET(i->first, &rdset))
-					break;
+					{
+						retSession = i->second;
+						break;
+					}
+					++i;
 				}
-				++i;
 			}
 		}
 	}

src/net/receivebuffer.h

@@ -0,0 +1,40 @@
+/***************************************************************************
+ *   Copyright (C) 2007 by Lothar May                                      *
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ *   This program is distributed in the hope that it will be useful,       *
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
+ *   GNU General Public License for more details.                          *
+ *                                                                         *
+ *   You should have received a copy of the GNU General Public License     *
+ *   along with this program; if not, write to the                         *
+ *   Free Software Foundation, Inc.,                                       *
+ *   59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.             *
+ ***************************************************************************/
+/* Buffer for ReceiveHelper. */
+
+#ifndef _RECEIVEBUFFER_H_
+#define _RECEIVEBUFFER_H_
+
+#include <net/netpacket.h>
+#include <list>
+
+// MUST be larger than MAX_PACKET_SIZE
+#define RECV_BUF_SIZE		2 * MAX_PACKET_SIZE
+
+typedef std::list<boost::shared_ptr<NetPacket> > NetPacketList;
+
+struct ReceiveBuffer
+{
+	ReceiveBuffer() : recvBufUsed(0) {}
+	NetPacketList					receivedPackets;
+	char							recvBuf[RECV_BUF_SIZE];
+	unsigned						recvBufUsed;
+};
+
+#endif

src/net/receiverhelper.h

@@ -23,15 +23,13 @@
 
 #include <net/socket_helper.h>
 #include <net/netpacket.h>
+#include <net/receivebuffer.h>
 
 #include <deque>
 #include <boost/shared_ptr.hpp>
 
-// MUST be larger than MAX_PACKET_SIZE
-#define RECV_BUF_SIZE		10 * MAX_PACKET_SIZE
 #define RECV_TIMEOUT_MSEC	50
 
-
 class ReceiverHelper
 {
 public:
@@ -41,17 +39,10 @@ public:
 	// Set the socket from which to receive data.
 	void Init(SOCKET socket);
 
-	boost::shared_ptr<NetPacket> Recv(SOCKET sock);
+	boost::shared_ptr<NetPacket> Recv(SOCKET sock, ReceiveBuffer &buf);
 
 protected:
-	boost::shared_ptr<NetPacket> InternalGetPacket();
+	void InternalGetPackets(ReceiveBuffer &buf);
-
-private:
-
-	SOCKET m_socket;
-
-	char m_tmpInBuf[RECV_BUF_SIZE];
-	unsigned m_tmpInBufSize;
 };
 
 #endif

src/net/sessiondata.h

@@ -22,6 +22,7 @@
 #define _SESSIONDATA_H_
 
 #include <net/socket_helper.h>
+#include <net/receivebuffer.h>
 #include <string>
 
 #define SESSION_ID_INIT			0
@@ -49,11 +50,15 @@ public:
 	void SetClientAddr(const std::string &addr)
 	{m_clientAddr = addr;}
 
+	ReceiveBuffer &GetReceiveBuffer()
+	{return m_receiveBuffer;}
+
 private:
 	SOCKET							m_sockfd;
 	unsigned						m_id;
 	State							m_state;
 	std::string						m_clientAddr;
+	ReceiveBuffer					m_receiveBuffer;
 };
 
 #endif