TFTP-сервер — проблема с многопоточной версией

Я создал простой tftp-сервер, который обрабатывает только запросы на чтение (RRQ). Все работало нормально, пока я не начал делать многопоточную версию сервера. В приложении я просто получаю запросы в основном потоке, а затем перенаправляю запрос в новый поток, который выполняет анализ пакетов. Поэтому мне нужно перенаправить сокет, полученный пакет и структуру sockaddr_in, содержащую информацию о клиенте, в поток. С учетом сказанного я создал структуру, которая содержит все это и пересылает их в pthread.

Я подозреваю, что проблема заключается в части инициализации и пересылки структуры clientThread; так как я уверен в правильности обработки внутри connection_handler.

Примечание. Для проверки можно использовать клиент tftp, поставляемый с Linux.

Вот код, который я написал до сих пор (потоковая версия). Пожалуйста, скомпилируйте его с флагом -pthread...

#include <stdio.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <pthread.h>
#include <time.h>

#define TIMEOUT 5000
#define RETRIES 3

void *connection_handler(void *);

struct clientThread 
{
    int clientSock;
    char buffer[1024];  
    struct sockaddr_in client;
};

int main()
{
    char buffer[1024];
    int udpSocket, client_socket, nBytes;
    struct sockaddr_in serverAddr, client;
    socklen_t addr_size;

    udpSocket = socket(AF_INET, SOCK_DGRAM, 0);

    serverAddr.sin_family = AF_INET;
    serverAddr.sin_port = htons(69);
    serverAddr.sin_addr.s_addr = inet_addr("127.0.0.1");
    memset(serverAddr.sin_zero, '\0', sizeof serverAddr.sin_zero); 

    bind(udpSocket, (struct sockaddr *) &serverAddr, sizeof(serverAddr));

    while(1)
    {
        memset(buffer, 0, 1024);
        nBytes = recvfrom(udpSocket,buffer,1024,0,(struct sockaddr *)&client, &addr_size);

        // Creating a thread and passing the packet received, the socket and the sockaddr_in struct...
        pthread_t client_thread;
        struct clientThread clientT;
        strcpy(clientT.buffer,buffer);
        clientT.clientSock = udpSocket;
        clientT.client = client;
        pthread_create(&client_thread, NULL, connection_handler, &clientT);
    }

    return 0;
}


void* connection_handler (void *clientThreaded)
{
        char buffer[1024], filename[200], mode[20], *bufindex, opcode;

        struct clientThread *cthread = clientThreaded;
        int udpSocket = cthread->clientSock;
        strcpy(buffer, cthread->buffer);
        struct sockaddr_in client = cthread->client;

        bufindex = buffer;
        bufindex++;

        // Extracting the opcode from the packet...
        opcode = *bufindex++;

        // Extracting the filename from the packet...
        strncpy(filename, bufindex, sizeof(filename)-1);

        bufindex += strlen(filename) + 1;

        // Extracting the mode from the packet...
        strncpy(mode, bufindex, sizeof(mode)-1);

        // If we received an RRQ...
        if (opcode == 1)
        {
                puts("Received RRQ Request");
                struct timeval tv;
                tv.tv_sec = 5;
                char path[70] = "tmp/";
                char filebuf [1024];
                int count = 0, i;  // Number of data portions sent
                unsigned char packetbuf[1024];
                char recvbuf[1024];
                socklen_t recv_size;

                socklen_t optionslength = sizeof(tv);
                setsockopt(udpSocket, SOL_SOCKET, SO_RCVTIMEO, &tv, optionslength);

                FILE *fp;
                char fullpath[200];
                strcpy(fullpath, path);
                strncat(fullpath, filename, sizeof(fullpath) -1);
                fp = fopen(fullpath, "r");
                if (fp == NULL)
                    perror("");

                memset(filebuf, 0, sizeof(filebuf));


                while (1)               
                {
                        int acked = 0;
                        int ssize = fread(filebuf, 1 , 512, fp);
                        count++;
                        sprintf((char *) packetbuf, "%c%c%c%c", 0x00, 0x03, 0x00, 0x00);
                        memcpy((char *) packetbuf + 4, filebuf, ssize);
                        packetbuf[2] = (count & 0xFF00) >> 8;
                        packetbuf[3] = (count & 0x00FF);

                        int len = 4 + ssize;

                        memset(recvbuf, 0, 1024);
                        printf("\nSending Packet #%i", count);
                        sendto(udpSocket, packetbuf, len, 0, (struct sockaddr *) &client, sizeof(client));

                        for (i=0; i<RETRIES; i++)
                        {
                            int result = recvfrom(udpSocket, recvbuf, 1024, 0, (struct sockaddr *) &client, &recv_size);

                                if ((result == -1) && ((errno == EAGAIN) || (errno == EWOULDBLOCK)))
                                {
                                    sendto(udpSocket, packetbuf, len, 0, (struct sockaddr *) &client, sizeof(client));
                                    printf("\nRetransmitting Packet #%i", count);
                                }

                                else if (result == -1)
                                {
                                   // Handle Error
                                }

                                else
                                {
                                    acked++;
                                        printf("\nReceived ACK For Data Packet #%i", count);
                                        break;
                                }
                    }


                        if (acked!=1)
                        {
                            puts("\nGave Up Transmission After 3 Retries");
                                break;
                        }

                        if (ssize != 512)
                            break;
                }
    }

    return 0;
}

Вот мой код для версии без резьбы...

#include <stdio.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <time.h>
#define TIMEOUT 5000
#define RETRIES 3

int main()
{
    int udpSocket, nBytes;
    char buffer[1024], filename[200], mode[20], *bufindex, opcode;
    struct sockaddr_in serverAddr, client;
    struct sockaddr_storage serverStorage;
    socklen_t addr_size;

    udpSocket = socket(AF_INET, SOCK_DGRAM, 0);

    serverAddr.sin_family = AF_INET;
    serverAddr.sin_port = htons(69);
    serverAddr.sin_addr.s_addr = inet_addr("127.0.0.1");
    memset(serverAddr.sin_zero, '\0', sizeof serverAddr.sin_zero); 

    bind(udpSocket, (struct sockaddr *) &serverAddr, sizeof(serverAddr));

    while(1)
    {
        memset(buffer, 0, 1024);
        nBytes = recvfrom(udpSocket,buffer,1024,0,(struct sockaddr *)&client, &addr_size);
        printf("%s",buffer);
        bufindex = buffer;
        bufindex++;

        // Extracting the opcode from the packet...     
        opcode = *bufindex++;

        // Extracting the filename from the packet...
        strncpy(filename, bufindex, sizeof(filename)-1);

        bufindex += strlen(filename) + 1;

        // Extracting the mode from the packet...       
        strncpy(mode, bufindex, sizeof(mode)-1);

        // If we received an RRQ...
        if (opcode == 1)
        {
                puts("Received RRQ Request");
                struct timeval tv;
                tv.tv_sec = 5;
                char path[70] = "tmp/";
                char filebuf [1024];
                int count = 0, i;  // Number of data portions sent
                unsigned char packetbuf[1024];
                char recvbuf[1024];
                socklen_t recv_size;

                socklen_t optionslength = sizeof(tv);
                setsockopt(udpSocket, SOL_SOCKET, SO_RCVTIMEO, &tv, optionslength);

                FILE *fp;
                char fullpath[200];
                strcpy(fullpath, path);
                strncat(fullpath, filename, sizeof(fullpath) -1);
                fp = fopen(fullpath, "r");
                if (fp == NULL)
                        perror("");

                 memset(filebuf, 0, sizeof(filebuf));


                while (1)
                {
                        int acked = 0;
                        int ssize = fread(filebuf, 1 , 512, fp);
                        count++;
                        sprintf((char *) packetbuf, "%c%c%c%c", 0x00, 0x03, 0x00, 0x00);
                        memcpy((char *) packetbuf + 4, filebuf, ssize);
                        packetbuf[2] = (count & 0xFF00) >> 8;
                        packetbuf[3] = (count & 0x00FF);

                        int len = 4 + ssize;

                        memset(recvbuf, 0, 1024);
                        printf("\nSending Packet #%i", count);
                        sendto(udpSocket, packetbuf, len, 0, (struct sockaddr *) &client, sizeof(client));

                        for (i=0; i<RETRIES; i++)
                        {
                                int result = recvfrom(udpSocket, recvbuf, 1024, 0, (struct sockaddr *) &client, &recv_size);

                                if ((result == -1) && ((errno == EAGAIN) || (errno == EWOULDBLOCK)))
                                {
                                    sendto(udpSocket, packetbuf, len, 0, (struct sockaddr *) &client, sizeof(client));
                        printf("\nRetransmitting Packet #%i", count);
                                }

                                else if (result == -1)
                                {
                                        // Handle Error
                                }

                                else
                                {
                                        acked++;
                        printf("\nReceived ACK For Data Packet #%i", count);
                                        break;
                                }
                        }

                         if (acked!=1)
                        {
                            puts("\nGave Up Transmission After 3 Retries");
                            break;
                        }

                        if (ssize != 512)
                                break;
                }
        }
    }   
    return 0;
}

Заранее спасибо :)


person user3490561    schedule 15.04.2015    source источник
comment
Без какой-либо блокировки на сокете? Как вы можете ожидать, что это сработает. Сокет является общим ресурсом, если вы делаете его многопоточным.   -  person Philip Stuyck    schedule 15.04.2015


Ответы (2)


Вы закольцовываете прослушивание порта 69, но фактическая передача данных будет осуществляться с другого случайно выбранного порта (пожалуйста, прочтите RFC 1350). Затем ваш основной цикл должен создавать новый поток для каждой новой передачи, этот новый поток должен получать структуру, содержащую путь к файлу для обслуживания, IP-адрес/порт назначения, случайно выбранный локальный порт и т. д.

Что вы должны учитывать при передаче указателя структуры в поток, так это память, поддерживающая структуру. В твоем случае

     struct clientThread clientT;

динамически создается в стеке, и, конечно, структура «отбрасывается», когда блок кода выходит за рамки (в вашем случае в каждом цикле), что означает, что вы передаете указатель на «скоро станет мусором» просто созданная ветка.

Я рекомендую использовать malloc/free при передаче структур только что созданным потокам.

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

Как видите, реализовать сервер даже для такого простого протокола, как TFTP, не так-то просто.

person Pat    schedule 15.04.2015

Ваш обработчик соединения здесь неверен, потому что в сокете, который вы передаете каждому потоку, нет никакой блокировки.

Большинство серверов, основанных на udp, на самом деле не разветвляют несколько потоков. TCP-серверы могут это сделать, потому что с каждым accept вы получаете новый сокет, который можно делегировать новому потоку, который не будет использоваться никаким другим потоком.

Но для udp вы в основном используете один и тот же сокет для всех своих потоков, и это не нормально. Если вы должны обеспечить защиту сокета, вы можете заставить его работать, но вы потеряете преимущества, которые вы пытаетесь получить, сделав его многопоточным.

person Philip Stuyck    schedule 15.04.2015
comment
Извините: я проголосовал за этот ответ; 1) Я считаю, что большинство серверов, основанных на udp, на самом деле не разветвляют несколько потоков. совершенно неверно при реализации, например, TFTP-сервер или отложенный DHCP-сервер и т. д. 2) Проблема блокировки сокета здесь не является реальной проблемой, поскольку неправильно считать, что OP будет использовать один и тот же сокет для всех потоков. ; пожалуйста, прочтите RFC-1350. - person Pat; 15.04.2015
comment
@ Пэт, я думаю, что 1 - это вопрос мнения. UDP-серверы, которые мы реализуем, являются однопоточными. Он даже упоминается у Стивенса. 2. вы видите в коде, что он на самом деле использует тот же сокет. В итоге, с этим кодом много проблем. - person Philip Stuyck; 15.04.2015
comment
stackoverflow.com/ questions/14041291/ также применим здесь - person Philip Stuyck; 15.04.2015
comment
@Филип; 1) Это не вопрос мнений; вы просто не можете реализовать TFTP-сервер без многопоточности, вы не можете реализовать отложенный DHCP-сервер без многопоточности. 2) вы указали на проблему совместного использования сокетов, когда должны были сказать, что OP на самом деле ошибочно интерпретирует стандарт TFTP. Проблема совместного использования сокетов естественным образом избегается при правильной реализации RFC 1350. - person Pat; 15.04.2015
comment
мы согласны не согласиться по всем пунктам :-) это не потому, что вы приводите пример из 2, что большинство udp-серверов внезапно становятся многопоточными. Я был прав с моей многопоточной проблемой и неправильным использованием общего сокета. Я только посмотрел на это под другим углом, чем ваш. Но твой ответ лучше моего, не могу не признать. - person Philip Stuyck; 15.04.2015
comment
Кстати, спасибо за объяснение отрицательного голоса. По крайней мере, я могу извлечь уроки из опыта, просто поставив отрицательный голос, я бы не знал ваших рассуждений. - person Philip Stuyck; 15.04.2015
comment
Филип, в вашем ответе есть ложная ассоциация, а в цитируемой ссылке говорится, что UDP и многопоточность плохо сочетаются, и это просто неправильно. VOIP-трафик также является ярким примером. Надеюсь, вы это видите. До встречи. - person Pat; 15.04.2015