0

I'm writing a simple quote server in C and running on Linux. It should pick a random quote from a text file and then start two threads:

  • the first is in charge of accepting incoming connections and respond with the selected quote,
  • the second should check once an hour if a day has passed. If it detects that a new day has started it should randomly pick another quote.

My problem is that while the connection thread works fine, the other doesn't even start.

To confirm this I've tried to add a debug print right at the start of the function executed by the thread and another inside it's while loop and none gets printed (those are removed from the code shown here).

I've also added some code to check the pthread_create() return value copied from it's man page but I'm unsure if it's actually working since it doesn't detect any error.

I've tied to first start the "timer thread" first and not start the connection thread at all but still it doesn't get executed. Here follows the relevant code, you can find the full source on GitHub:

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

#define handle_error_en(en, msg) \
               do { errno = en; perror(msg); exit(EXIT_FAILURE); } while (0)

pthread_mutex_t quoteLock=PTHREAD_MUTEX_INITIALIZER;
pthread_t checkForNewDayThread, connectionHandlerThread;

void * timer_thread_code(){ //The thread will act as a timer checking every hour if a day has passed
    while (true) {
        sleep(3600);
        if (a_day_has_passed()) {
            pthread_mutex_lock(&quoteLock);
            QOTD = read_random_quote_from_file(pathToQOTDfile);
            pthread_mutex_unlock(&quoteLock);
        }
    }
}

void * connection_thread_code(int port){    //Code for the thread to handle connections
    struct sockaddr_in address;
    int server_fd, new_socket, opt = 1, addrlen = sizeof(address);

    // Creating socket file descriptor
    if ((server_fd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
    {
        perror("socket failed");
        exit(EXIT_FAILURE);
    }

    // Forcefully attaching socket to the port 1717
    if (setsockopt(server_fd, SOL_SOCKET, SO_REUSEADDR | SO_REUSEPORT, &opt, sizeof(opt))==-1)
    {
        perror("setsockopt");
        exit(EXIT_FAILURE);
    }
    address.sin_family = AF_INET;
    address.sin_addr.s_addr = INADDR_ANY;
    address.sin_port = htons( port );

    // Forcefully attaching socket to the port 1717
    if (bind(server_fd, (struct sockaddr *)&address, sizeof(address))<0)
    {
        perror("bind failed");
        exit(EXIT_FAILURE);
    }
    if (listen(server_fd, 100) < 0)
    {
        perror("listen");
        exit(EXIT_FAILURE);
    }

    printf("Listening on port %i\n", port);

    while(1) {  //connection handler loop
        if ((new_socket = accept(server_fd, (struct sockaddr *) &address, (socklen_t *) &addrlen)) < 0) {
            perror("accept");
            exit(EXIT_FAILURE);
        }
        pthread_mutex_lock(&quoteLock);
        send(new_socket, QOTD, strlen(QOTD), 0);
        pthread_mutex_unlock(&quoteLock);
        close(new_socket);
    }
}

int main(int argc, char const *argv[])
{
    int thread1, thread2, join;
    thread1=pthread_create(&connectionHandlerThread, NULL, connection_thread_code(port), NULL);
    if(thread1!=0) handle_error_en(thread1, "pthread_create");
    thread2=pthread_create(&checkForNewDayThread, NULL, timer_thread_code(), NULL);
    if(thread2!=0) handle_error_en(thread2, "pthread_create");
    join=pthread_join(connectionHandlerThread, NULL);
    if(join!=0) handle_error_en(join, "pthread_join");
    return 0;
}

Note: I put only one pthread_join() because both threads should run forever, so to prevent returning from main it should suffice to join only one of the two.

Thanks in advance to anyone who helps me

Sander De Dycker
  • 16,053
  • 1
  • 35
  • 40
GTP95
  • 13
  • 4
  • 1
    The third argument to `pthread_create` is a function to serve as the thread routine, but instead you are _invoking_ your thread routines and passing their return value in as that third argument. Please whittle all this down to a [minimum, reproducible example](https://stackoverflow.com/help/minimal-reproducible-example). – pilcrow Feb 27 '20 at 14:44

1 Answers1

1

When you call pthread_create like in your code :

thread1=pthread_create(&connectionHandlerThread, NULL, connection_thread_code(port), NULL);

You actually first call connection_thread_code(port) on the main thread, and pass the return value of that function call as parameter to pthread_create.

That's not what you want to do. You want to pass the function pointer as parameter to pthread_create, as well as the arguments to pass to it :

int port = 1234; /* <-- make sure this is in scope for the duration of the thread ! */
thread1 = pthread_create(&connectionHandlerThread, NULL, connection_thread_code, &port);
/* ... */
pthread_join(connectionHandlerThread, NULL);

For this to work, your thread function should have this signature :

void* connection_thread_code(void* port_ptr);

And get the port like so :

int port = *((int*) port_ptr);

Read up more on passing an int parameter to a thread function here.

Similarly for the second thread (pass the function pointer instead of calling the function, and change the signature).

Sander De Dycker
  • 16,053
  • 1
  • 35
  • 40
  • You're right, thanks! I've fixed my code and now it works. I'm only left wondering why the connection thread was working even if the code was wrong, but maybe it's architecture-dependend, since I've tested it on a raspberry pi and there both threads weren't being executed. – GTP95 Feb 27 '20 at 20:30
  • @GTP95 : you called `connection_thread_code(port)` from the main thread, so your main thread was serving as the connection thread. – Sander De Dycker Feb 28 '20 at 07:11