Этот многопоточный tcp сервер C корректен?

Question

Я создал многопоточный C-сервер TCP.Кажется, это работает (как клиент, я набираю сообщение, и сообщение отправляется на сервер, и сервер печатает то, что клиент отправил в потоке (и отправляет обратно идентификатор клиента).

Я уважаю"лучшие практики" многопоточного TCP-сервера C? Может быть, мне следует использовать семафор для доступа / использования переменной client_counter?

#include <stdlib.h> 
#include <stdio.h> 
#include <string.h>

#include <unistd.h> // disable close() warning
#include <sys/socket.h> 
#include <sys/types.h> 
#include <netinet/in.h> 

#include <pthread.h>

#define MAX_CONNECTIONS 5

static int client_counter = 0;

void* serverWorker(void* context)
{
    char client_response[256];

    int sock = *(int*)context;
    char message[256]  = "\n Hello dear client, you are the client number \n";
    char numero[12];
    sprintf(numero, "%d", client_counter); // SHOULD I USE A SEMAPHORE HERE FOR client_counter ?

    while(1)
    {
        memset(client_response, 0, sizeof(client_response)); // clean string
        recv(sock, &client_response, sizeof(client_response), 0);
        printf("client number %s sent: '%s' \n", numero, client_response);
        if (send(sock, numero , strlen(numero) , 0) < 0)
        {
            printf("ERROR while sending response to client from worker \n");
        }
    }
    return NULL;
}



int main() 
{ 

    printf("Waiting for incoming connections ...\n");

    // socket creation 
    int server_socket;
    server_socket = socket(AF_INET, SOCK_STREAM, 0);

    // dserver address
    struct sockaddr_in server_address;
    server_address.sin_family = AF_INET;
    server_address.sin_port = htons(9002);
    server_address.sin_addr.s_addr = INADDR_ANY;

    // bind the socket to IP and port
    bind(server_socket, (struct sockaddr*) &server_address, sizeof(server_address));

    listen(server_socket, MAX_CONNECTIONS);

    int client_socket;
    while((client_socket = accept(server_socket, NULL ,NULL)))
    {
        client_counter++; 
        pthread_t thread_id;
        pthread_create(&thread_id, NULL, serverWorker, (void*)&client_socket);
        printf("new client ! \n");
    }

    close(server_socket);

    return 0; 
} 

Answer 1

Одной из распространенных ошибок является то, что вы не проверяете возвращаемые значения вызовов send и recv.Эти вызовы могут отправлять и получать меньше, чем весь буфер, и такие случаи должны обрабатываться, а также отключаться.Это также избавит от необходимости использовать memset и strlen в полученных данных.

Как правило, выделение потока каждому клиенту считается немасштабируемым.Возможно, вам захочется прочитать знаменитую проблему C10K для хорошего подхода к стратегиям ввода-вывода для обработки многих клиентов.Статья старая, но совет не подвластен времени.

Answer 2

В вашем коде есть несколько проблем ... вы создаете поток на входящем соединении и передаете всем созданным потокам ссылку (ту же ссылку) на переменную, в которой вы сохранили дескриптор сокета.Это заставит все потоки использовать одну и ту же переменную для хранения всех дескрипторов сокетов, которые вы получите из подстановочного знака.Вероятно, вы думаете, хорошо, я делаю копию только при запуске потока, так что этого не произойдет , но подумайте, что два соединения, которые приходят почти одновременно, поток main() работает и обрабатывает оба.Затем первый и второй потоки планируются, и оба получают одинаковый сохраненный дескриптор (второй), и первое соединение просачивается.

Другое дело, что хотя эта переменная является локальной для главной, он перестанет существовать, как только main() вернется (что не конец программы, если потоки должны выжить после main() return), но, как вы находитесь в бесконечном цикле (вы, вероятно,не знаю, но единственное средство для server_socket, чтобы сообщить об ошибке, - это если вы уничтожаете (close() it) в потоке или отбрасываете интерфейс, к которому он подключен.) Это может привести к SIGSEGV trap.

Вы можете свободно передавать значение int, приведенное к (void *), без проблем, так как функция тела потока преобразует его обратно в int перед использованием, что вообще сводит к нулю, так как типы указателей обычно больше по размеру (или равны, но не меньше), чем int.В любом случае, это строго неопределенное поведение , но, вероятно, это сработает (поскольку унаследованное программное обеспечение полно таких преобразований, поэтому все компиляторы обычно реализуют, чтобы попытаться соблюдать это). Правильный способ сделать это - объявитьstruct информации, передаваемой потоку при запуске и возвращающемся из него.Затем вы можете хранить на нем все, что захотите, но подумайте, что, поскольку у вас есть динамическое количество потоков, вам нужно динамически распределить структуры.

В отношении использования client_counterпеременная, единственная нить, касающаяся этой переменной, это та, в которой выполняется код main().Это создает большую проблему, чем риск, представленный выше, два обновления в быстрой последовательности могут заставить оба потока обновлять значения в main после того, как main произведет оба обновления.

Другая проблема заключается в том, что вам нужно объявить это volatile, поскольку код потока не будет предполагать, что он изменяется только между доступами и, вероятно, будет кэшировать его как переменную регистра.

Сообщения, передаваемые между main() и различными полученными вами потоками, могут быть реализованы.двумя способами.Это является причиной того, что процедуры получают void * на входе и возвращают void * при возврате:

• Первый использует динамические struct локальных данных (malloc() ed, переданный из main() в поток и обратно при завершении (когда вы присоединяете поток к основному). Этот способ позволяет вам собирать информацию о результате из потока в main, а затем вам нужно free(3) structв основном. Структура используется как коммуникационное сообщение между потоком и основной подпрограммой в обоих направлениях, и вы можете хранить там любую информацию, которую вам нужно передать или вернуть обратно. После завершения потока вы можете free()структура в основном (не делайте этого в потоке, поскольку он должен пережить свою смерть)

• Второе больше не требует связи с main(), и потоки должны освободить структурупосле его завершения. Это проще и более адекватно вашему примеру. Таким образом, вы можете уничтожить структуру в потоке или в основном, но только если вы уже присоединились ки уверены, что структура не будет использоваться им.