Go Premium for a chance to win a PS4. Enter to Win

x
?
Solved

Problem receiving message from socket.

Posted on 2010-09-21
8
Medium Priority
?
531 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

0
Comment
Question by:PDamasceno
8 Comments
 
LVL 32

Expert Comment

by:phoffric
ID: 33728070
Would you be able to include enough files to allow us to build the projects?
0
 

Author Comment

by:PDamasceno
ID: 33728478
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.
0
 
LVL 35

Expert Comment

by:Duncan Roe
ID: 33730047
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
0
Concerto's Cloud Advisory Services

Want to avoid the missteps to gaining all the benefits of the cloud? Learn more about the different assessment options from our Cloud Advisory team.

 
LVL 35

Expert Comment

by:Duncan Roe
ID: 33730055
Oops! See man 7 tcp for the Nagle algorithm
0
 
LVL 36

Accepted Solution

by:
mccarl earned 2000 total points
ID: 33730282
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)
0
 

Author Comment

by:PDamasceno
ID: 33733429
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.
0
 

Author Comment

by:PDamasceno
ID: 33734416
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

0
 

Author Comment

by:PDamasceno
ID: 33736635
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.
0

Featured Post

Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Question has a verified solution.

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

Summary: This tutorial covers some basics of pointer, pointer arithmetic and function pointer. What is a pointer: A pointer is a variable which holds an address. This address might be address of another variable/address of devices/address of fu…
Windows programmers of the C/C++ variety, how many of you realise that since Window 9x Microsoft has been lying to you about what constitutes Unicode (http://en.wikipedia.org/wiki/Unicode)? They will have you believe that Unicode requires you to use…
Video by: Grant
The goal of this video is to provide viewers with basic examples to understand and use while-loops in the C programming language.
The goal of this video is to provide viewers with basic examples to understand and use conditional statements in the C programming language.

927 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question