From ecab6ed86bdbfb95fc9bebbb93e988852b85c643 Mon Sep 17 00:00:00 2001 From: lotodore Date: Mon, 3 Sep 2007 16:35:45 +0000 Subject: [PATCH] Fixed a really nasty receive bug (TCP only). Using SCTP would be so much better... --- src/net/clientcontext.h | 6 +- src/net/common/clientstate.cpp | 3 +- src/net/common/receiverhelper.cpp | 82 ++++++++++++++-------------- src/net/common/servergamestate.cpp | 2 +- src/net/common/serverlobbythread.cpp | 2 +- src/net/common/sessionmanager.cpp | 57 +++++++++++-------- src/net/receivebuffer.h | 40 ++++++++++++++ src/net/receiverhelper.h | 15 +---- src/net/sessiondata.h | 5 ++ 9 files changed, 132 insertions(+), 80 deletions(-) create mode 100644 src/net/receivebuffer.h diff --git a/src/net/clientcontext.h b/src/net/clientcontext.h index 83e22fda..c7eb51bf 100644 --- a/src/net/clientcontext.h +++ b/src/net/clientcontext.h @@ -22,7 +22,7 @@ #define _CLIENTCONTEXT_H_ #include - +#include 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 diff --git a/src/net/common/clientstate.cpp b/src/net/common/clientstate.cpp index 6738b2cd..a2c2c693 100644 --- a/src/net/common/clientstate.cpp +++ b/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 tmpPacket = client.GetReceiver().Recv(client.GetContext().GetSocket()); + boost::shared_ptr tmpPacket = + client.GetReceiver().Recv(client.GetContext().GetSocket(), client.GetContext().GetReceiveBuffer()); if (tmpPacket.get()) { diff --git a/src/net/common/receiverhelper.cpp b/src/net/common/receiverhelper.cpp index c3a3eb84..b745b049 100644 --- a/src/net/common/receiverhelper.cpp +++ b/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 -ReceiverHelper::Recv(SOCKET sock) +ReceiverHelper::Recv(SOCKET sock, ReceiveBuffer &buf) { - boost::shared_ptr tmpPacket(InternalGetPacket()); - - if (!tmpPacket.get()) + if (buf.receivedPackets.empty()) { - 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; - tmpPacket = InternalGetPacket(); + buf.recvBufUsed += bytesRecvd; + InternalGetPackets(buf); } } } } - return tmpPacket; -} - -boost::shared_ptr -ReceiverHelper::InternalGetPacket() -{ boost::shared_ptr tmpPacket; - - // 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) + if (!buf.receivedPackets.empty()) { - try - { - // 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. - } + tmpPacket = buf.receivedPackets.front(); + buf.receivedPackets.pop_front(); } return tmpPacket; } +void +ReceiverHelper::InternalGetPackets(ReceiveBuffer &buf) +{ + bool dataAvailable = true; + do + { + boost::shared_ptr 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); +} + diff --git a/src/net/common/servergamestate.cpp b/src/net/common/servergamestate.cpp index a5dfb397..b72e70e6 100644 --- a/src/net/common/servergamestate.cpp +++ b/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); diff --git a/src/net/common/serverlobbythread.cpp b/src/net/common/serverlobbythread.cpp index 7136eb62..68aed0d5 100644 --- a/src/net/common/serverlobbythread.cpp +++ b/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. diff --git a/src/net/common/sessionmanager.cpp b/src/net/common/sessionmanager.cpp index eabc8c4b..d3501669 100644 --- a/src/net/common/sessionmanager.cpp +++ b/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 - } - 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)) + if (maxSock == INVALID_SOCKET) { - 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. - boost::mutex::scoped_lock lock(m_sessionMapMutex); - SessionMap::iterator i = m_sessionMap.begin(); - SessionMap::iterator end = m_sessionMap.end(); - - while (i != end) + // 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)) { - 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; - break; + if (FD_ISSET(i->first, &rdset)) + { + retSession = i->second; + break; + } + ++i; } - ++i; } } } diff --git a/src/net/receivebuffer.h b/src/net/receivebuffer.h new file mode 100644 index 00000000..8e027329 --- /dev/null +++ b/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 +#include + +// MUST be larger than MAX_PACKET_SIZE +#define RECV_BUF_SIZE 2 * MAX_PACKET_SIZE + +typedef std::list > NetPacketList; + +struct ReceiveBuffer +{ + ReceiveBuffer() : recvBufUsed(0) {} + NetPacketList receivedPackets; + char recvBuf[RECV_BUF_SIZE]; + unsigned recvBufUsed; +}; + +#endif diff --git a/src/net/receiverhelper.h b/src/net/receiverhelper.h index cddeb813..f0387313 100644 --- a/src/net/receiverhelper.h +++ b/src/net/receiverhelper.h @@ -23,15 +23,13 @@ #include #include +#include #include #include -// 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 Recv(SOCKET sock); + boost::shared_ptr Recv(SOCKET sock, ReceiveBuffer &buf); protected: - boost::shared_ptr InternalGetPacket(); - -private: - - SOCKET m_socket; - - char m_tmpInBuf[RECV_BUF_SIZE]; - unsigned m_tmpInBufSize; + void InternalGetPackets(ReceiveBuffer &buf); }; #endif diff --git a/src/net/sessiondata.h b/src/net/sessiondata.h index 94faa6a5..c8f184b0 100644 --- a/src/net/sessiondata.h +++ b/src/net/sessiondata.h @@ -22,6 +22,7 @@ #define _SESSIONDATA_H_ #include +#include #include #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