Improve company productivity with a Business Account.Sign Up

Status: Solved
Priority: Medium
Security: Public
Views: 534
Last Modified: 2012-05-10

Problem receiving message from socket.

Hey Experts.

I have this chat program that is being a pain to finish, I'm on a linux machine.
My problem this time is when I receive a message from the socket in a ClientHandler class. The message is not complete when I receive it, don't know why. The send( ) from the socket library isn't supposed to send all the message?
What I send is this: 3#14:41#tom
It is a string message serialized with a Pack( ) function I've made.
But what the ClientHandler class receives is only this: 3#14
I've tried increasing the buffer length and a couple of things but with no use at all.
I've spent quite some time trying to get around this on my own, so now I come to you guys.
The codes to the related classes are attached.
If you need to see any more code from my program just ask.
Thanks in advance.

Sender.cpp

#include "include/Sender.h"
#include "include/ChatMessage.h"
#include "include/AttentionMessage.h"
#include "include/ServerMessage.h"
#include "include/Socket.h"
#include "include/Client.h"
#include <string>
#include <vector>
#include <iostream>


Sender::Sender( Socket socket )
{
   pthread_create(&m_thread, 0, &Sender::StartThread, this);
   dataSocket = socket;
}

Sender::~Sender( )
{
}

std::string Sender::GetUsername( )
{
   return m_username;
}

void Sender::run( )
{
   std::string packedData;

   std::cout << "Client/Server Chat" << std::endl;
   std::cout << "Type the username: ";
   std::getline( std::cin, m_username );

   ServerMessage smsg;
   smsg.SetUsername( m_username );
   smsg.SetTime( );
   packedData = smsg.Pack( );
   dataSocket.SendData( packedData );

   while( m_chatMessage != "/q" )
   {
      std::getline( std::cin, m_chatMessage );

      if( m_chatMessage == "/a" )
      {
         AttentionMessage attmsg;
         attmsg.SetUsername( m_username );
         attmsg.SetTime( );
         packedData = attmsg.Pack( );
         dataSocket.SendData( packedData );
      }

      ChatMessage msg;
      msg.SetMessage( m_chatMessage );
      msg.SetUsername( m_username );
      msg.SetTime( );
      packedData = msg.Pack( );
      dataSocket.SendData( packedData );
   }

   std::cout << "Bye." << std::endl;
   dataSocket.Close( );
}

void * Sender::StartThread( void *arg )
{
   reinterpret_cast<Sender *>(arg)->run( );
}

Open in new window

ClientHandler.cpp

#include "include/ClientHandler.h"
#include "include/Socket.h"
#include "include/ServerDispatcher.h"
#include "include/ChatMessage.h"
#include "include/AttentionMessage.h"
#include "include/ServerMessage.h"
#include <string>
#include <vector>
#include <exception>
#include <iostream>
#include <pthread.h>

ClientHandler::ClientHandler( Socket socket )
{
   pthread_create(&m_thread, 0, &ClientHandler::StartThread, this);
   dataSocket = socket;
}

ClientHandler::~ClientHandler( )
{
}

void ClientHandler::SendMessage( std::string packedMessage )
{
   m_ClientMessageQueue.push_front( packedMessage );
   std::cout << "Message added to ClientHandler queue." << std::endl;
}

std::string ClientHandler::GetMessageFromQueue( )
{
   while ( true )
   {
      if ( m_ClientMessageQueue.size( ) != 0 )
      {
         break;
      }
   }

   std::string message = m_ClientMessageQueue.back( );
   m_ClientMessageQueue.pop_back( );
   return message;
}

void ClientHandler::run( )
{
   char buffer[1024];
   std::string message;
   std::string packedMessage;
   std::string rawMessage;
   unsigned int result;

   while ( true )
   {
      try
      {
         if ( dataSocket.DataCheck( ) > 0 )
         {
            try
            {
               if ( !dataSocket.RecvData( buffer ) )
               {
                  break;
               }

               packedMessage = buffer;
               result = packedMessage.find( "1" );
               if ( result != std::string::npos && result < 2 )
               {
                  ChatMessage msg;
                  msg.Unpack( packedMessage );
                  rawMessage = "[" + msg.GetTime( ) + "] " + msg.GetUsername( ) + ": " + msg.GetMessage( );
                  ServerDispatcher::Instance( ).DispatchMessage( rawMessage );
               }

               result = packedMessage.find( "2" );
               if ( result != std::string::npos && result < 2 )
               {
                  AttentionMessage attmsg;
                  attmsg.Unpack( packedMessage );
                  rawMessage = "[" + attmsg.GetTime( ) + "] " + attmsg.GetUsername( ) + " is asking for the ATTENTION of all chat members!!!";
                  ServerDispatcher::Instance( ).DispatchMessage( rawMessage );
               }

               result = packedMessage.find( "3" );
               if ( result != std::string::npos && result < 2 )
               {
                  ServerMessage smsg;
                  smsg.Unpack( packedMessage );
                  rawMessage = "[" + smsg.GetTime( ) + "] " + "SERVER: " + smsg.GetUsername( ) + " has logged in.";
                  ServerDispatcher::Instance( ).DispatchMessage( rawMessage );
               }

               memset( &buffer[0], 0, sizeof(buffer) );
            }
            catch ( std::exception &e )
            {
               std::cerr << "Connection is broken." << std::endl;
               break;
            }
         }

         message = GetMessageFromQueue( );
         if ( !dataSocket.SendData( message ) )
         {
            break;
         }
      }
      catch( std::exception &e )
      {
         std::cerr << "Connection is broken." << std::endl;
         break;
      }
   }

   dataSocket.Close( );
   ServerDispatcher::Instance( ).DeleteClient( this );
}

void * ClientHandler::StartThread( void * arg )
{
   reinterpret_cast<ClientHandler *>(arg)->run( );
}

Open in new window

Socket.cpp

#include "include/Socket.h"
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/select.h>
#include <netdb.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include <cerrno>
#include <string>
#include <iostream>

Socket::Socket( )
{
}

Socket::~Socket( )
{
}

void Socket::Listen( const int port )
{
   int yes = 1;
   struct sockaddr_in address;

   //Create socket and check if it worked
   if ( (m_socketFD = socket(PF_INET, SOCK_STREAM, 0)) < 0 )
   {
      std::cerr << "Error creating socket :: " << strerror( errno ) << std::endl;
      return;
   }

   //Set socket to allow multiple connections
   if ( setsockopt(m_socketFD, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(int)) < 0 )
   {
      std::cerr << "Error setting socket option :: " << strerror( errno ) << std::endl;
      return;
   }

   address.sin_family = PF_INET;
   address.sin_addr.s_addr = INADDR_ANY;
   address.sin_port = htons( port );

   //Bind socket to the specified port
   if ( bind( m_socketFD, (struct sockaddr *)&address, sizeof( address )) < 0 )
   {
      std::cerr << "Error binding socket to port :: " << strerror( errno ) << std::endl;
      return;
   }

   //Listen for connection calls
   if ( listen( m_socketFD, 20 ) < 0 )
   {
      std::cerr << "Error listening for connections :: " << strerror( errno ) << std::endl;
      return;
   }
}

void Socket::Connect( const char *hostname, const int port )
{
   struct sockaddr_in address;

   //Create socket and check if it worked
   if ( (m_socketFD = socket(PF_INET, SOCK_STREAM, 0)) < 0 )
   {
      std::cerr << "Error creating socket :: " << strerror( errno ) << std::endl;
      return;
   }

   address.sin_family = PF_INET;
   address.sin_port = htons( port );
   address.sin_addr.s_addr = inet_addr( hostname );

   if ( connect( m_socketFD, (struct sockaddr *)&address, sizeof( address ) ) < 0 )
   {
      std::cerr << "Error connecting to server :: " << strerror( errno ) << std::endl;
      return;
   }
}

bool Socket::Accept( Socket& socket )
{
   struct sockaddr_in address;
   int FD;

   socklen_t addrlen = sizeof( address );

   //Accept incoming connections
   FD = accept( m_socketFD, (struct sockaddr *)&address, &addrlen );
   if ( FD < 0 )
   {
      std::cerr << "Error accepting connection :: " << strerror( errno ) << std::endl;
      return false;
   }

   socket.SetSocketFD( FD );
   return true;
}

void Socket::Close( )
{
   //Close socket
   if ( close( m_socketFD ) < 0 )
   {
      std::cerr << "Error closing socket :: " << strerror( errno ) << std::endl;
      return;
   }
}

bool Socket::SendData( std::string packedMessage )
{
   const char *message = packedMessage.c_str( );
   int nbytes;

   //Send packed message to socket
   nbytes = send( m_socketFD, message, sizeof( message ), 0 );
   if ( nbytes == -1 )
   {
      std::cerr << "Error sending to socket :: " << strerror( errno ) << std::endl;
      return false;
   }

   return true;
}

bool Socket::RecvData( char *buffer )
{
   int nbytes;

   //Get packed message from socket
   nbytes = recv( m_socketFD, buffer, sizeof( buffer ), 0 );
   if ( nbytes <= 0 )
   {
      if ( nbytes == 0 )
      {
         std::cerr << "Connection was closed." << std::endl;
         return false;
      }
      else
      {
         std::cerr << "Error receiving from socket :: " << strerror( errno ) << std::endl;
         return false;
      }
   }

   return true;
}

int Socket::DataCheck( )
{
   fd_set readfds;
   int retval;

   FD_ZERO( &readfds );
   FD_SET( m_socketFD, &readfds );

   retval = select ( m_socketFD + 1, &readfds, NULL, NULL, NULL );

   return retval;
}

int Socket::GetSocketFD( )
{
   return m_socketFD;
}

void Socket::SetSocketFD( int FD )
{
   m_socketFD = FD;
}

Open in new window

PDamasceno

Asked: 2010-09-21

Advertise Here

1 Solution

LVL 33

phoffricCommented: 2010-09-21

Would you be able to include enough files to allow us to build the projects?

PDamascenoAuthor Commented: 2010-09-21

I'll do that.
But I'll have to include all my program.
I'll make a pack and send it to a file sharing site for you, it isn't that big.
But I'm out of work now, so tomorrow I give you this.
Thanks for a quick response.

LVL 35

Duncan RoeSoftware DeveloperCommented: 2010-09-21

I think I can see what might be happening. You only make 1 recv() all and expect the entire message. In general with TCP sockets that isn't going to happen. See man 7 socket and search for the string "Nagle". O_NDELAY will help somewhat. SO_LINGER will help a lot more - with both you will probably fluke a message per recv() most of the time. TCP is a streaming protocol though, not a messaging one so more than 1 recv() may be necessary anyway.
But another fault is that you close the socket after sending. That can lose outgoing data. SO_LINGER should fix that - see man 7 socket

LVL 35

Duncan RoeSoftware DeveloperCommented: 2010-09-21

Oops! See man 7 tcp for the Nagle algorithm

LVL 36

mccarlIT Business Systems Analyst / Software DeveloperCommented: 2010-09-21

First things first, you are creating your receive buffer in ClientHandler::run() with a size of 1024, but then you are passing this buffer to your Socket::RecvData() without passing the size too. You are trying to do a sizeof(buffer) inside RecvData but since this is now just a char* the sizeof() returns 4 (the number of bytes used for a pointer) and hence you are only receiving 4 bytes of your message. Pass the size of the buffer as well as the actual buffer to your RecvData() call.

However, I agree with all of duncan_roe's comments regarding TCP being a stream protocol, but I will add some further comments. Because it is a stream protocol you cannot guarantee that your message will START at the start of the buffer either. Depending on your semantics for sending and receiving this may not be much of an issue, but it may need to be considered. So to expand on what duncan mentioned about more than 1 recv() call, you need to continue to call recv() until you have received your entire message (it could take 1 call or 20 calls to recv(), you don't know). This obviously implies that you need some way of "knowing" when you have received your entire message. Two popular ways is to have a certain "marker" in the stream which indicates the end of a message (obviously it should be something that doesn't occur anywhere else in the message), and the other is to send the length of the message first and then you know how many characters to recv().

The other possibility is that you use UDP to communicate rather TCP, as this provides gives you packet semantics . However, UDP is an unreliable protocol (as in it doesn't guarantee delivery, you have to cater for retries, etc if you want reliability) and it also doesn't guarantee the order of packets (may or may not be an issue)

PDamascenoAuthor Commented: 2010-09-22

Here it is my whole project as asked by phoffric:
http://www.mediafire.com/file/gsd8xpwlforu27x/Chat_ClientServer.rar
I assure you that there isn't any virus in there, but feel free to scan the files.

I will try adding more recv calls as said by duncan_roe and mccarl. Also thanks for the depth explaining mccarl. Very enlightening.
I'll post a response soon.

PDamascenoAuthor Commented: 2010-09-22

I've added a msgSize and a bufferSize variable to the SendData and RecvData function calls and used them in the send( ) and recv( ) calls but the program still insist on sending or receiving (don't know where this happens anymore) 4 bytes only.
Why the hell this keep happening?
I know that I may have to call send or recv a couple of times to send or receive all the data, but I'm pretty sure that there isn't a 4 byte limit in send( ) and recv( ) calls. So what can I possibly be still doing wrong?

The alterations I've made are attached.

bool Socket::SendData( std::string packedMessage, int msgSize )
{
   const char *message = packedMessage.c_str( );
   int nbytes;

   //Send packed message to socket
   nbytes = send( m_socketFD, message, msgSize, 0 );
   if ( nbytes == -1 )
   {
      std::cerr << "Error sending to socket :: " << strerror( errno ) << std::endl;
      return false;
   }

   return true;
}

bool Socket::RecvData( char *buffer, int bufferSize )
{
   int nbytes;

   //Get packed message from socket
   nbytes = recv( m_socketFD, buffer, bufferSize, 0 );
   if ( nbytes <= 0 )
   {
      if ( nbytes == 0 )
      {
         std::cerr << "Connection was closed." << std::endl;
         return false;
      }
      else
      {
         std::cerr << "Error receiving from socket :: " << strerror( errno ) << std::endl;
         return false;
      }
   }

   return true;
}

Open in new window

PDamascenoAuthor Commented: 2010-09-22

I have figured it out what the problem was.
duncan_roe e mccarl pointed to the problems inside my sendData and recvData functions, wich was the fact that I was using sizeof on the pointer, returning the size of the pointer and not the size of the buffer. The same thing was happening in the sendData function, I was getting the size of the point to string and not the size of the message.

The other problem was with wrong usage of sizeof again, this time was in the Sender class where I get the messages from the user. I have a variable that stores the messages typed by the user: std::string message, I was using sizeof( message ) and wasn't getting the size of the message stored in the variable.

So this is it. Wrong sizeof usage. Beware, sizeof( ) is evil.
Heh.
But it has it's usages.

Thanks duncan_roe and mccarl.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Expand your skills with this month's free Premium course.

Problem receiving message from socket.

Join & Write a Comment Already a member? Login.

Featured Post