Liseł Liseł - 2 months ago 5
C Question

Client does not receive message on newly formed connection

So I have quite simple multiple server-client application (Linux). What I am trying to do for starters is to connect to the server receive the message from it and write message to the server which then is echos.

The problem is that on connection instead of greetings message from server, client is ready to type the message ito the terminal - there is no greetings message. After typing whatever message I want I get echoed back with the greetings message! What am I doing wrong here? Code below.

server.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#include <pthread.h>

#include <netdb.h>
#include <netinet/in.h>

int server(int client_socket);
void* handle_connection(void* inc);

int main(int argc, char* argv[])
{
int socket_fd;
int client_socket_fd;
int port_number;
int client_length;
int * new_sock;
struct sockaddr_in serv_addr;
struct sockaddr_in cli_addr;
char buffer[256];

socket_fd = socket(AF_INET, SOCK_STREAM, 0);

if(socket_fd < 0) {
perror("Error opening socket!");
exit(1);
}

/* Initialize socket structure */
port_number = 1234;
bzero((char *) &serv_addr, sizeof(serv_addr));
serv_addr.sin_family = AF_INET;
serv_addr.sin_addr.s_addr= INADDR_ANY;
serv_addr.sin_port = htons(port_number);

/* Binding */
if(bind(socket_fd, (struct sockaddr *) &serv_addr, sizeof(serv_addr)) < 0) {
perror("Error on binding!");
exit(1);
}

listen(socket_fd, 10);
client_length = sizeof(cli_addr);

while(client_socket_fd = accept(socket_fd, (struct sockaddr*) &cli_addr, &client_length)) {
pthread_t thread_id;
new_sock = malloc(sizeof(client_socket_fd));
*new_sock = client_socket_fd;

if(pthread_create(&thread_id, NULL, handle_connection, (void*) new_sock)) {
perror("could not create thread");
exit(1);
}
pthread_join(thread_id, NULL);
}

if(client_socket_fd < 0) {
perror("Error on accept client");
exit(1);
}

return 0;
}

void* handle_connection(void* inc)
{
int socket_fd = *(int *) inc;
int message_size;
char *message;
char buffer[256];

message = "You have been connected\n";
write(socket_fd, message, strlen(message));
message = "I will repeat what you type\n";
write(socket_fd, message, strlen(message));

while((message_size = recv(socket_fd, buffer, sizeof(buffer)/sizeof(buffer[0]), 0)) > 0) {
write(socket_fd, buffer, strlen(buffer));
}

if(message_size == 0) {
puts("Client disconnected");
fflush(stdout);
}
else if(message_size == -1) {
perror("Reading back from client failed");
}

free(inc);

return 0;

}


client.c

#include <stdio.h>
#include <stdlib.h>
#include <netdb.h>
#include <netinet/in.h>
#include <string.h>
#include <pthread.h>

int main(int argc, char* argv[])
{
int socket_fd, port_number;
struct sockaddr_in serv_addr;
struct hostent *server;
char buffer[256];

if(argc<2) {
fprintf(stderr,"Incorrect arguments input\n");
exit(0);
}

//port_number = atoi(argv[2]);
port_number = 1234;

server = gethostbyname(argv[1]);

socket_fd = socket(AF_INET, SOCK_STREAM, 0);
if(socket_fd < 0) {
perror("Error opening socket");
exit(1);
}

bzero((char*) &serv_addr, sizeof(serv_addr));
serv_addr.sin_family = AF_INET;
serv_addr.sin_port = htons(port_number);
bcopy((char*) server->h_addr,(char *) &serv_addr.sin_addr.s_addr,sizeof(server->h_length));

if(connect(socket_fd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) {
perror("Error connecting");
exit(1);
}
while(1) {
bzero(buffer, sizeof(buffer)/sizeof(buffer[0]));
printf("Enter the message: ");
bzero(buffer, sizeof(buffer)/sizeof(buffer[0]));
fgets(buffer, sizeof(buffer)/sizeof(buffer[0]) - 1, stdin);

if(send(socket_fd, buffer, strlen(buffer), 0) < 0) {
perror("Error sending message");
exit(1);
}

bzero(buffer, sizeof(buffer)/sizeof(buffer[0]));
if(recv(socket_fd, buffer, sizeof(buffer), 0) < 0) {
perror("Error reading back from server");
exit(1);
}

printf("Server reply: ");
printf("%s",buffer);
}
close(socket_fd);
return 0;
}


Here is the unwanted result of the program:
unwatned result

Server reply should in the example: hej.

Answer

The problem is that on connection instead of greetings message from server, client is ready to type the message ito the terminal - there is no greetings message.

This is because you're reading the user input before fetching the greetings with recv() and showing them the the user.

After typing whatever message I want I get echoed back with the greetings message!

This is because you are fetching/showing the greetings after calling fgets().

There is a number of errors in the code. I'll point to the biggest ones.

  1. You're calling pthread_join() right after pthread_create(). That's blocking new connections(i.e. further accept()s). This should be done outside of the loop. In this case, you need some storage for thread IDs. For example:
const int thread_pool_step = 10;
int thread_pool_size = thread_pool_step;
pthread_t *thread_ids = malloc(thread_pool_size * sizeof(pthread_t));
memset(thread_ids, 0, thread_pool_size * sizeof(pthread_t));
int thread_index = -1;

while (!do_shutdown)
  {
    /* ... */
    if (++thread_index >= thread_pool_size)
      {
        thread_pool_size += thread_pool_step;
        thread_ids = realloc(thread_ids, thread_pool_size * sizeof(pthread_t));
        if (thread_ids == NULL)
          {
            fprintf(stderr, "failed to realloc thread pool: %s\n",
              strerror(errno));
            abort();
          }
      }

    pthread_create(&thread_ids[thread_index++], NULL,
                   handle_connection, (void*) new_sock);
  }

while (--thread_index >= 0)
  {
    if (thread_ids[thread_index])
      pthread_join(thread_ids[thread_index], NULL);
  }

if (thread_ids != NULL)
  free(thread_ids);
  1. As mentioned in the comments to the question, you need some kind of protocol. For example, you might read the greetings until the server sends a line starting from "READY" string. The client might remember the server state, then proceed with the chat logic:
enum {
  INIT_STATE,
  READY_STATE
} serv_state = INIT_STATE;

#define check_serv_state(str, st) \
  (strncmp(str, #st, sizeof(#st) - 1) == 0)

for (;;)
{
  bzero(buffer, sizeof(buffer)/sizeof(buffer[0]));
  rc = recv(socket_fd, buffer, sizeof(buffer), 0);
  /* ... */

  if (serv_state == INIT_STATE)
    {
      char *part = strtok(buffer, "\n");
      do
        {
          if (check_serv_state(part, READY))
            {
              printf("The server is ready for the chat\n");
              serv_state = READY_STATE;
            }
          else
            printf("Server: [[[%s]]]\n", part);
        }
      while ((part = strtok(NULL, "\n")));

      if (serv_state != READY_STATE)
        continue;
    }
  else
    printf("Server reply: %s", buffer);

  /* ... */
  printf("Enter the message: ");
  /* ... */
}
  1. I'd also suggest implementing some signal handling for graceful shutdown. At least, for the server:
volatile sig_atomic_t do_shutdown = 0; /* global */

static void
sigterm_handler(int sig)
{
  printf("Caught signal %d. Leaving...\n", sig);
  do_shutdown = 1;
}

/* ... */

struct sigaction a;
a.sa_handler = sigterm_handler;
a.sa_flags = 0;
sigemptyset(&a.sa_mask);
sigaction(SIGINT,  &a, NULL);
sigaction(SIGQUIT, &a, NULL);
sigaction(SIGTERM, &a, NULL);

/* Check do_shutdown in the loops */
while (!do_shutdown) { /* ...recv(), accept(), etc. */ }

Note, that accessing globals from asynchronous contexts(signal handlers, thread handlers[especially]) is usually unsafe without the use of mutual exclusion.

Below is the full code with the fixes applied:

server.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#include <pthread.h>
#include <errno.h>
#include <signal.h>
#include <netdb.h>
#include <netinet/in.h>

volatile sig_atomic_t do_shutdown = 0;

static void
sigterm_handler(int sig)
{
  printf("Caught signal %d. Leaving...\n", sig);
  do_shutdown = 1;
}


static void
send_message(int socket_fd, const char *message)
{
  int rc;
  int message_size = strlen(message);

  do
    {
      rc = send(socket_fd, message, message_size, 0);
      if (rc == -1)
        {
          fprintf(stderr, "send() failed: %s\n", strerror(errno));
          abort();
        }
    }
  while (rc < message_size && !do_shutdown);
}


static void *
handle_connection(void *inc)
{
  int socket_fd = *(int *)inc;
  int message_size;
  char *message;
  char buffer[256];

  message = "You have been connected\n";
  send_message(socket_fd, message);

  message = "I will repeat what you type\n";
  send_message(socket_fd, message);

  message = "READY\n";
  send_message(socket_fd, message);

  while (!do_shutdown)
    {
      memset(buffer, 0, sizeof(buffer));
      message_size = recv(socket_fd, buffer, sizeof(buffer) / sizeof(buffer[0]), 0);
      if (message_size > 0)
        send_message(socket_fd, buffer);
      else if (message_size == -1)
        {
          fprintf(stderr, "recv() failed: %s\n", strerror(errno));
          break;
        }
      else if (message_size == 0)
        {
          puts("Client disconnected");
          fflush(stdout);
          break;
        }
    }

  free(inc);

  return 0;
}


int
main(int argc, char* argv[])
{
  int socket_fd;
  int client_socket_fd;
  int port_number;
  socklen_t client_length;
  int *new_sock;
  struct sockaddr_in serv_addr;
  struct sockaddr_in cli_addr;

  socket_fd = socket(AF_INET, SOCK_STREAM, 0);
  if (socket_fd < 0)
    {
      perror("Error opening socket!");
      exit(1);
    }

  /* Initialize socket structure */
  port_number = 12345;
  bzero((char *)&serv_addr, sizeof(serv_addr));
  serv_addr.sin_family = AF_INET;
  serv_addr.sin_addr.s_addr= INADDR_ANY;
  serv_addr.sin_port = htons(port_number);

  /* Binding */
  if (bind(socket_fd, (struct sockaddr *) &serv_addr,  sizeof(serv_addr)) < 0)
    {
      perror("Error on binding!");
      exit(1);
    }

  const int thread_pool_step = 10;
  listen(socket_fd, thread_pool_step);
  client_length = sizeof(cli_addr);

  int thread_pool_size = thread_pool_step;
  pthread_t *thread_ids = malloc(thread_pool_size * sizeof(pthread_t));
  if (thread_ids == NULL)
    {
      perror("Failed to allocate memory for thread ids");
      abort();
    }
  memset(thread_ids, 0, thread_pool_size * sizeof(pthread_t));
  int thread_index = -1;

  struct sigaction a;
  a.sa_handler = sigterm_handler;
  a.sa_flags = 0;
  sigemptyset(&a.sa_mask);
  sigaction(SIGINT,  &a, NULL);
  sigaction(SIGQUIT, &a, NULL);
  sigaction(SIGTERM, &a, NULL);

  while (!do_shutdown)
    {
      client_socket_fd = accept(socket_fd, (struct sockaddr *)(&cli_addr), &client_length);
      if (client_socket_fd == -1)
        {
          fprintf(stderr, "accept() failed: %s\n", strerror(errno));
          break;
        }

      new_sock = malloc(sizeof(client_socket_fd));
      *new_sock = client_socket_fd;

      if (++thread_index >= thread_pool_size)
        {
          thread_pool_size += thread_pool_step;
          thread_ids = realloc(thread_ids, thread_pool_size * sizeof(pthread_t));
          if (thread_ids == NULL)
            {
              fprintf(stderr, "failed to realloc thread pool: %s\n", strerror(errno));
              abort();
            }
        }

      if (pthread_create(&thread_ids[thread_index++], NULL,
                         handle_connection, (void*) new_sock))
        {
          fprintf(stderr, "pthread_create() failed: %s\n", strerror(errno));
          abort();
        }
    }

  puts("Waiting for threads to finish");
  while (--thread_index >= 0)
    {
      if (thread_ids[thread_index])
        pthread_join(thread_ids[thread_index], NULL);
    }

  if (thread_ids != NULL)
    free(thread_ids);

  return 0;
}

client.c

#include <stdio.h>
#include <stdlib.h>
#include <netdb.h>
#include <netinet/in.h>
#include <string.h>
#include <pthread.h>
#include <unistd.h>

int main(int argc, char* argv[])
{
  int socket_fd, port_number;
  struct sockaddr_in serv_addr;
  struct hostent *server;
  char buffer[256];
  int rc;

  if (argc<2)
    {
      fprintf(stderr,"Incorrect arguments input\n");
      exit(0);
    }

  port_number = 12345;

  server = gethostbyname(argv[1]);

  socket_fd = socket(AF_INET, SOCK_STREAM, 0);
  if (socket_fd < 0)
    {
      perror("Error opening socket");
      exit(1);
    }

  bzero((char*) &serv_addr, sizeof(serv_addr));
  serv_addr.sin_family = AF_INET;
  serv_addr.sin_port = htons(port_number);
  bcopy((char*) server->h_addr,(char *)
        &serv_addr.sin_addr.s_addr,sizeof(server->h_length));

  if (connect(socket_fd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0)
    {
      perror("Error connecting");
      exit(1);
    }

  enum
    {
      INIT_STATE,
      READY_STATE
    } serv_state = INIT_STATE;

#define check_serv_state(str, st) \
  (strncmp(str, #st, sizeof(#st) - 1) == 0)

  for (;;)
    {
      bzero(buffer, sizeof(buffer)/sizeof(buffer[0]));
      rc = recv(socket_fd, buffer, sizeof(buffer), 0);
      if (rc < 0)
        {
          perror("Error reading back from server");
          exit(1);
        }
      else if (rc == 0)
        {
          printf("The server has been disconnected. Quitting.\n");
          exit(0);
        }

      if (serv_state == INIT_STATE)
        {
          char *part = strtok(buffer, "\n");
          do
            {
              if (check_serv_state(part, READY))
                {
                  printf("The server is ready for the chat\n");
                  serv_state = READY_STATE;
                }
              else
                printf("Server: [[[%s]]]\n", part);
            }
          while ((part = strtok(NULL, "\n")));

          if (serv_state != READY_STATE)
            continue;
        }
      else
        printf("Server reply: %s", buffer);

      bzero(buffer, sizeof(buffer)/sizeof(buffer[0]));
      printf("Enter the message: ");
      bzero(buffer, sizeof(buffer)/sizeof(buffer[0]));
      fgets(buffer, sizeof(buffer)/sizeof(buffer[0]) - 1, stdin);

      if (send(socket_fd, buffer, strlen(buffer), 0) < 0)
        {
          perror("Error sending message");
          exit(1);
        }
    }
  close(socket_fd);
  return 0;
}