Solved

Problem receiving message from socket.

Posted on 2010-09-21
8
518 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 34

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
 
LVL 34

Expert Comment

by:Duncan Roe
ID: 33730055
Oops! See man 7 tcp for the Nagle algorithm
0
How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

 
LVL 35

Accepted Solution

by:
mccarl earned 500 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

How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

Join & Write a Comment

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…
This is a short and sweet, but (hopefully) to the point article. There seems to be some fundamental misunderstanding about the function prototype for the "main" function in C and C++, more specifically what type this function should return. I see so…
The goal of this video is to provide viewers with basic examples to understand recursion in the C programming language.
The goal of this video is to provide viewers with basic examples to understand opening and reading files in the C programming language.

708 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

Need Help in Real-Time?

Connect with top rated Experts

18 Experts available now in Live!

Get 1:1 Help Now