Can't share variables between pthreads, values are different

Question

I'm trying to program a web server, using a thread pool, where the main thread takes a connection, passes it to a thread, and the thread processes it.

I have a struct for each thread, and a workQueue to hold them

struct worker {
    pthread_t* thread;
    struct queue* workerQueue;
    char* busy;
    int connfd;
    int id;
};

struct queue {
    int start;
    int end;
    int size;
    struct worker* workers;
};

The main thread sets up the queue and threads, and loops over connections

  struct queue* workerQueue;
            workerQueue = (struct queue*) constructQueue(10);
            int j;
            int* testParam;
            //create workers and put in queue
            for(j=0;j<5;j++)
            {
                struct worker* w = &(workerQueue->workers[j]);
                w = (struct worker*)constructWorker(processConnection,testParam, workerQueue,j);
                queueAdd(workerQueue,w);
            }
connection = accept(fd, (struct sockaddr *) &cliaddr, &cliaddrlen);
        puts("got connection\n");


         w =(struct worker*) queueRemove(workerQueue);
        //w->connfd = connection;
        w->busy = "BUSY";
        printf("Worker %d has accepted a connection and is %s\n",w->id,w->busy);

using these two functions..

   struct queue* constructQueue(int numThreads)
    {
        struct queue* q = (struct queue *)malloc(sizeof(struct queue));
        q->start = 0;
        q->end = 0;
        q->workers = (struct worker* )malloc(sizeof(struct worker)*numThreads);
        q->size = numThreads;
        return q;
    }

struct worker* constructWorker(void* (*function)(void*),void* param, struct queue* wq, int i)
{
    struct worker* w = (struct worker*)malloc(sizeof(struct worker));
    w->workerQueue = wq;
    char * busy = (char*)malloc(10);
    w->busy= "IDLE";
    w->connfd = 0;
    w->id = i;
    pthread_t t;
    w->thread = &t; 
    pthread_create(w->thread,NULL,function,w);
    return w;
}

...and the function the threads use is

void* processConnection(void* serverThread)
{
        //cast serverthread
        struct worker* w;
        char* b;
        int threadID;
        w = (struct worker*)serverThread;
        b = w->busy;
        threadID = w->id;


        while (1)
        {
            char c[10];
            printf("\nbusy: %s, thread: %d\n",b,threadID);
            gets(c)

;

What I would like to happen is: the workers get created, with busy set to IDLE, and begin busy waiting. then in the main loop, a connection is accepted and assigned to a worker, and the workers busy value is set to BUSY. then in processConnections, if a thread is busy, it should actually process it. The problem is, although my queue contains pointers not value, when I update the workers in the main thread, it doesnt seem to affect the value of the workers in processConnection. I can set busy to BUSY and have it print that out in the main loop, but the value of busy is always IDLE in processConnection. Any ideas?

Answer 1

i would suggest adding 2 mutexes one at worker and one at queue

   struct worker {
      pthread_t* thread;
      struct queue* workerQueue;
      mutex_t QueueMutex;
      char* busy;
      int connfd;
      int id;
      };`

   struct queue {
       int start;
       int end;
       int size;
       mutex_t workermutex;
       struct worker* workers;
   };

Your code should work like below

When you creating a new socket, have a lock on the workermutex and then assign an connection

The worker threads each time locks the QueueMutex and adds/deletes data from the queue for processing .

Answer 2

Try changing definition of busy to

volatile char * busy;

This tells the compiler that the value of this variable can change while the code is running, even if the code isn't accessing that variable explicitly.

However, you have many other problems. For example,

char * busy = (char*)malloc(10);
w->busy= "IDLE";

will leak memory that has been allocated by malloc. Don't try to use strings for keeping track of state. Define an enum {IDLE, BUSY} instead and define busy to be a variable of that type.

Answer 3

You probably aren't seeing the updated value in the other thread because there is no synchronization point between the threads. Compiler optimizations and cache (in)consistency are two reasons why this happens. To keep the same strategy you need a memory barrier. If you are using gcc, the easiest way is to put __sync_synchronize() before reading the shared data and after writing the shared data.

Another thing you need to fix is when you do

pthread_t t;
w->thread = &t;

The memory of t is liable to be reused after that function returns. You cannot take the address of a local variable and store it in a way that will outlive the lifetime of the function. The proper thing to do is have a pthread_t field in struct worker and pass the address of the field to pthread_create:

pthread_create(&w->thread, ...);