I created a simple tftp server that only handles read requests (RRQ). Everything was working fine until I started to make a multi-threaded version of the server. In the application, I simply receive requests in the main thread and I then forward the request to a new thread that does the packet analysis. Therefore, I need to forward the socket, the received packet and the sockaddr_in struct that contains the client information to the thread. With that said, I created a struct that holds all of these and forward them to the pthread.
I get two identical error messages, one in the main and the other in the connection handler. The problems seems to be in the referencing these variables in the struct and retrieving them in the thread. It seems the problem is in the following statements: in connection_handler(): buffer = cthread->buffer; and in the main(): clientT.buffer = buffer;
Here's the code, I've written so far...
#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>
#define TIMEOUT 5000
#define RETRIES 3
void *connection_handler(void *);
struct clientThread
{
int clientSock;
char opcode;
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);
//opcode = cthread->opcode;
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;
}
Thanks in advance :)
In the main loop, the variable clientT is local inside that loop, once the loop iterates the variable will go out of scope and any pointer to it will become invalid. Dereferencing such a pointer will lead to undefined behavior.
Instead what you should to is to dynamically allocate the structure using malloc, and pass that pointer instead. Don't forget to free the structure once you're done with it in the thread.