My TCP client seems to have a race condition with my server!

Hello people

I have been trying to make an educational TCP server-client model in c++, using the standard c socket libraries.

for full reference and a GitHub - stefmitropoulos/simple_c_tcp_client: Simple TCP Client in c++c-make file, you can visit the code for the server here and for the client here GitHub - stefmitropoulos/simplesocketserver: Simple C++ socket server.
I am using spdlog library for logging, used less than 10 times though. If you don't want to download it from GitHub - gabime/spdlog: Fast C++ logging library. just remove it in the code



What I have done is:

Server Side :
Create a socket, bind, listen to it and accept when there is a pending connection. After that, spawn a thread to handle the connection and detach it.
The thread sends a welcome message to the client, receives a message from the client and returns the message with the Caesar Cipher applied to it.

Client Side :
Create a socket and connect to the server. Receive the message and then send a message to the server. Then receive the ciphered text.

My problem lies (I think) in the client side.

Code:

---


char buff[MAX];

if (recv(sockfd, buff, sizeof(buff), 0) < 0)
{

exit(6);

}

puts(buff);

bzero(buff, sizeof(buff));

strcpy(buff, "PIZZA");

if (send(sockfd, buff, sizeof(buff), 0) < 0) {

  exit(5);

}

struct pollfd fds[1];

fds[0].fd = sockfd;

fds[0].events = (POLLIN);

int ret = 1;

while (ret) {

  ret = poll(fds, 1, 2000);

  if (ret == -1) {

    spdlog::error("ERROR ON POLL");

    exit(5);

  }

  if (ret == 0) {

    spdlog::info("Timeout reached");

  }

  while (ret) {

    if (fds[0].revents == 0) {

      continue;

    }

    ret--;

    if (fds[0].revents & POLLIN) {  //NOLINT

      //INCOMING DATA

      bzero(buff, sizeof(buff));

      if (recv(sockfd, buff, 100, MSG_DONTWAIT) < 0) {

        puts("Recv()");

        exit(6);

      }

    }

  }

}

close(sockfd);

std::cout << "Reply from the server: " << buff << std::endl;

---

I am writing the buffer with 0s before receiving a final time, but when ran, this part prints nothing!

When I run it on the debugger, or if I induce a sleep() just before the recv(), I get \nQJAAB, the caesar cipher version of PIZZA!

It is driving me mad, any help would be appreciated!

I think a more "educational" way is the client send a request and the server send a response.

In essence this is what happens here. The only difference is the welcoming message on connect, isn't it? How would you go about this?

Here's a simple server. Test with telnet:

Quote:

---

$ telnet <targethost> 8080
...
hello<enter>

---

Code:

---

/* server.c

  Simple server, receive connections, checks for 'hello' and responds,
  closing the connection.

  Compile with:
    gcc -O2 -pthread -o server server.c -lpthread
*/

// Needed because we're using strerror_r, GNU style.
#define _GNU_SOURCE
#include <unistd.h>
#include <stdlib.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>

// Max, 16 simultaneous connections requests.
#define QUEUE_SIZE 16

static void *thread_routine( void * );

int main( void )
{
  // setup an efemeral port 8080 at ANY address.
  struct sockaddr_in sin = { .sin_port = htons(8080) };
  int fd; // socket file descriptor.

  // Create an stream TCP/IP socket.
  if ( ( fd = socket( AF_INET, SOCK_STREAM, IPPROTO_TCP ) ) < 0 )
  {
    perror( "socket" );
    return EXIT_FAILURE;
  }

  // before binding, set reuse addr flag for the socket.
  {
    // for solaris change this to char and set to '1' (0x31).
    int n = 1;
    setsockopt( fd, SOL_SOCKET, SO_REUSEADDR, &n, sizeof n );
  }

  // bind the address of this server socket to the descriptor.
  if ( bind( fd, (struct sockaddr *)&sin, sizeof sin ) )
  {
    perror( "bind" );
    close( fd );
    return EXIT_FAILURE;
  }

  // Puts the socket in listening mode.
  if ( listen( fd, QUEUE_SIZE ) )
  {
    perror( "listen" );
    close( fd );
    return EXIT_FAILURE;
  }

  fputs( "Waiting connections...\n", stderr );

  // Accept connections and spawn threads...
  while ( 1 )
  {
    struct sockaddr_in sin_remote;
    socklen_t size = sizeof sin_remote;
    int conn_fd;  // connection socket descriptor.

    // accept will block if there is no incomming connection.
    if ( ( conn_fd = accept( fd, &sin_remote, &size ) ) > 0 )
    {
      pthread_t tid;
      pthread_attr_t attr;
      char buffer[INET_ADDRSTRLEN];

      // Show accepted connection.
      inet_ntop( AF_INET, &sin_remote.sin_addr, buffer, sizeof buffer );
      fprintf( stderr, "Connection from %s:%hu\n", buffer, ntohs(sin_remote.sin_port) );

      // Spawn a new detached thread.
      pthread_attr_init( &attr );
      pthread_attr_setdetachstate( &attr, PTHREAD_CREATE_DETACHED );
      /* NOTE: setup the thread stack size if you want to limit them. */
      //pthread_attr_setstacksize( &attr, _SC_THREAD_STACK_MIN );
      pthread_create( &tid, &attr, thread_routine, &conn_fd );
      pthread_attr_destroy( &attr );
    }
    else
    {
      // 256 bytes should be enough!
      char buffer[256];

      // strerror_r is thread safe!
      fprintf( stderr, "Error accepting connection: %s\n", strerror_r(errno, buffer, sizeof buffer) );
    }
  }

  // never gets here!
  return EXIT_SUCCESS;
}

// Out thread routine.
void *thread_routine( void *paramp )
{
  static char *data[] = { "Hello, professor Falken.\n"
                          "I don't want to play a nice game of chess right now.\n\n",

                          "What?!\n"
                          "Cannot understand you. Bye!\n\n" };
  char *p;
  char buffer[33] = { 0 };
  ssize_t size;
  int fd;

  fd = *(int *)paramp;

  // SIMPLE server... I'm not testing for disconnections (size == 0)
  // or insufficient data...
  if ( ( size = recv( fd, buffer, sizeof buffer - 1, 0 ) ) > 0 )
  {
    // The client will send "GET" followed or not by \r or \n
    p = buffer + size - 1;
    while ( p >= buffer && ( *p == '\r' || *p == '\n' ) )
      *p-- = '\0';

    // shortcut to:
    //  if ( ! strcasecmp( buffer, "hello" )
    //    p = data[0];
    //  else
    //    p = data[1];
    p = data[ !! strcasecmp( buffer, "hello" ) ];

    send( fd, p, strlen( p ) - 1, 0 );
  }

  close( fd );

  // fputs() is MT-Safe!
  fputs( "Connection closed.\n", stderr );

  return NULL;
}

---

Woah, thank you for the effort and time! This is indeed very nice :wink:. To clarify, when I said "Educational purposes" I meant to educate myself, not others. It seems you have experience on this, it is very well written indeed!

I am not sure I understand this though :/

Quote:

---

p = data[ !! strcasecmp( buffer, "hello" ) ];

---

Regardless of opinion, have you taken a look at the weird response in my code?

Thanks... Maybe the confusion is abour the expression:

Code:

---

!! strcasecmp( buffer, "hello" )

---

Here strcasecmp() will return 0 is the strings are equal (case insensitive); -1 or 1 if not.
The operation ! is a boolean negation (NOT) where the operand is consider as false if 0, true otherwhise, so, if strcasecmp() return 0 the fist ! will turn it to 1.
By the standard BOOLEAN RESULTS are always 0 or 1, so !! is a way to make sure we'll have only 0 or 1 as result.

If buffer == "hello" we'll get 0, if not we'll get 1. Since there is only 2 pointers in data array: data[0] and data[1], this is easier to do than the if (listed in the comment).

Got it?

A great tutorial on how to deal with sockets you can find here: Beej Guide to Network Programming.
This covers IPv6 as well... and the language used by the author is very interesting (full of jokes!)...

Quote:

---

Here strcasecmp() will return 0 is the strings are equal (case insensitive); -1 or 1 if not.
The operation ! is a boolean negation (NOT) where the operand is consider as false if 0, true otherwhise, so, if strcasecmp() return 0 the fist ! will turn it to 1.
By the standard BOOLEAN RESULTS are always 0 or 1, so !! is a way to make sure we'll have only 0 or 1 as result.

If buffer == "hello" we'll get 0, if not we'll get 1. Since there is only 2 pointers in data array: data[0] and data[1], this is easier to do than the if (listed in the comment).

Got it?

---

Aha!, so the double negation is to get rid of the -1 case!

Quote:

---

Originally Posted by flp1969

A great tutorial on how to deal with sockets you can find here: Beej Guide to Network Programming.
This covers IPv6 as well... and the language used by the author is very interesting (full of jokes!)...

---

This is where I started and found myself eager to try some of it on my own! Much of my code would be similar to Beej's. Apart from basics though, I have to dig deeper to get what I am doing wrong