Still celebrating National IT Professionals Day with 3 months of free Premium Membership. Use Code ITDAY17

Solved

Problem receiving message from socket.

Posted on 2010-09-21

1 solution

Medium Priority

529 Views

Last Modified: 2012-05-10

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

Comment

Question by:PDamasceno

[X]

Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

Help others & share knowledge
Earn cash & points
Learn & ask questions

Join The Community

8 Comments

LVL 32

Expert Comment

by:phoffric

ID: 337280702010-09-21

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

Author Comment

by:PDamasceno

ID: 337284782010-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

Expert Comment

by:Duncan Roe

ID: 337300472010-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

Expert Comment

by:Duncan Roe

ID: 337300552010-09-21

Oops! See man 7 tcp for the Nagle algorithm

LVL 36

Accepted Solution

by:mccarl

mccarl earned 2000 total points

ID: 337302822010-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)

Author Comment

by:PDamasceno

ID: 337334292010-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.

Author Comment

by:PDamasceno

ID: 337344162010-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

Author Comment

by:PDamasceno

ID: 337366352010-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.

Featured Post

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

If You Don't Need Interface Builder.

Article by: pgnatyuk

Preface I don't like visual development tools that are supposed to write a program for me. Even if it is Xcode and I can use Interface Builder. Yes, it is a perfect tool and has helped me a lot, mainly, in the beginning, when my programs were small…

A Simple Linux script to retrieve information from the web using xpath selection

Article by: pfrancois

Have you ever been frustrated by having to click seven times in order to retrieve a small bit of information from the web, always the same seven clicks, scrolling down and down until you reach your target? When you know the benefits of the command l…