3

I'm new to concept of threading. I was doing producer consumer problem in C but the consumer thread doesn't run when parallel with producer.

my code is as follows:

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

int S;
int E;
int F;

void waitS(){
    //printf("hbasd");
    while(S<=0);
    S--;
}

void signalS(){
    S++;
}

void waitE(){
    while(E<=0);
    E--;
}

void signalE(){
    E++;
}
void waitF(){
    while(F<=0);
    F--;
}

void signalF(){
    F++;
}
int p,c;

void* producer(void *n){
    int *j = (int *)n;
    int i = *j;
    while(1){
        waitS();
        waitE();
        printf("Producer %d\n",E);
        signalS();
        signalF();
        p++;
        if(p>=i){
            printf("Exiting: producer\n");
            pthread_exit(0);
        }
    }
}

void* consumer(void *n){
    int *j = (int *)n;
    int i = *j;
    while(1){
        waitS();
        waitF();
        printf("Consumer %d\n",E);
        signalS();
        signalE();
        c++;
        if(c>=i){
            printf("Exiting Consumer\n");
            pthread_exit(0);
        }
    }
}


int main(int argc, char* argv[]){

    int n = atoi(argv[1]);

    E = n;
    S = 1;
    F = 0;

    int pro = atoi(argv[2]);
    int con = atoi(argv[3]);


    pthread_t pid, cid;
    pthread_attr_t attr;

    pthread_attr_init(&attr);

    pthread_create(&pid,&attr,producer,(void *)&pro);
    pthread_create(&cid,&attr,consumer,(void *)&con);

    pthread_join(pid,NULL);
    pthread_join(cid,NULL);

}

When i give the input as ./a.out 3 4 3 i.e n=3, pro = 4, con = 3

I get no out just an a dead lock kind of situation.

I expect an output like

Producer 2 Producer 1 Producer 0 Consumer 0 Consumer 1 Producer 0 Exiting: producer Consumer 0 Exiting: consumer

...similar outputs where Producer runs 4 times and consumer thrice

When i give an input like ./a.out 4 4 3 i get the following output

Producer 3 Producer 2 Producer 1 Producer 0 Exiting: producer Consumer 0 Consumer 1 Consumer 2 Exiting: consumer

from the results i'm getting a conclusion that pthread producer is executing 1st and then is pthread consumer.

I want both of them to execute simultaneously so that i get an answer similar to the first expected output when test cases like 3 4 3 are given.

Andriy Berestovskyy
  • 8,059
  • 3
  • 17
  • 33
Prajval M
  • 2,298
  • 11
  • 32
  • Lot of things in here could mess up, for example, `global` variable `S` is accessed without any synchronization from both `producer` and `consumer` threads. – ViKiG Feb 01 '18 at 06:19
  • even if S is accessed, waitS() takes care of synchronisation – Prajval M Feb 01 '18 at 06:29
  • What is the goal of `waitS()`? – ViKiG Feb 01 '18 at 06:36
  • it's a sephamore – Prajval M Feb 01 '18 at 06:40
  • `semaphores` are much more complex than a simple `while` loop. As one of answer explains you will have to use either a `mutex` or `atomic-variables` or `semaphore` for read/write operations on those variables. – ViKiG Feb 01 '18 at 06:51

4 Answers4

2

You are accessing non-atomic variables from different threads without any kind of synchronization; this is a race condition and it leads to undefined behavior.

In particular, modern CPUs provide separate registers and separate caches to each CPU core, which means that if a thread running on CPU core #1 modifies the value of a variable, that modification may remain solely in CPU #1's cache for quite a while, without getting "pushed out" to RAM, and so another thread running on CPU core #2 may not "see" the thread #1's update for a long time (or perhaps never).

The traditional way to deal with this problem is either to serialize accesses to your shared variables with one or more mutexes (see pthread_mutex_init(), pthread_mutex_lock(), pthread_mutex_unlock(), etc), or use atomic variables rather than standard ints for values you want to access from multiple threads simultaneously. Both of those mechanisms have safeguards to ensure that undefined behavior won't occur (if you are using them correctly).

Jeremy Friesner
  • 70,199
  • 15
  • 131
  • 234
  • won't waitS() take care of synchronization part ?? – Prajval M Feb 01 '18 at 06:30
  • BTW won't mutex make producer run first and consumer second ?? – Prajval M Feb 01 '18 at 06:57
  • This should not happen – Prajval M Feb 01 '18 at 06:58
  • waitS() doesn't help because the compiler doesn't know to expect that the variable S might be modified at any time by another thread, and therefore is likely to cache a local copy of the value of S in a local register or core-specific cache; so even if another core did change the value of S, the running waitS() call would likely not see the change. – Jeremy Friesner Feb 01 '18 at 14:51
  • A mutex will only allow one thread to "hold" the mutex at a time -- i.e. if thread A has locked the mutex and thread B calls pthread_mutex_lock(), then thread B's pthread_mutex_lock() call won't return until thread A has called pthread_mutex_unlock(). Outside of that restriction, both threads can run simultaneously. Usually that isn't a problem since you try to make sure each thread holds the mutex for as little time as possible, but if it bothers you, you can avoid mutexes and use atomic variables instead. – Jeremy Friesner Feb 01 '18 at 14:54
  • @JeremyFriesner Hey, just a small remark. The statement "modification may remain solely in CPU #1's [...], so another thread running on CPU core #2 may not see the update for a long time (or perhaps never)." is not quite true, because the vast majority of modern multi-core architectures are cache coherent. This basically guarantees that changes in local cache are propagated to other local caches. More on that on Wikipedia: https://en.wikipedia.org/wiki/Cache_coherence – Andriy Berestovskyy Feb 06 '18 at 12:29
  • I stand by my use of the weasel-word "may" :) – Jeremy Friesner Feb 06 '18 at 18:44
2

You can not access same memory from two different threads without synchronization. The standard for pthreads spells it out quite clearly here:

Applications shall ensure that access to any memory location by more than one thread of control (threads or processes) is restricted such that no thread of control can read or modify a memory location while another thread of control may be modifying it. Such access is restricted using functions that synchronize thread execution and also synchronize memory with respect to other threads.

Besides, even if we ignore that many CPUs don't synchronise memory unless you explicitly ask them to, your code is still incorrect in normal C because if variables can be changed behind your back they should be volatile. But even though volatile might help on some CPUs, it is incorrect for pthreads.

Just use proper locking, don't spin on global variables, there are methods to heat a room that are much cheaper than using a CPU.

Art
  • 19,807
  • 1
  • 34
  • 60
1

In general, you should use synchronization primitives, but unlike other answerers I do believe we might not need any if we run this program on x86 architecture and prevent compiler to optimize some critical parts in the code.

According to Wikipedia, x86 architecture has almost sequential consistency, which is more than enough to implement a producer-consumer algorithm.

The rules to successfully implement such an producer-consumer algorithm is quite simple:

  1. We must avoid writing the same variable from different threads, i.e. if one thread writes to variable X, another thread just read from X
  2. We must tell the compiler explicitly that our variables might change somewhere, i.e. use volatile keyword on all shared between threads variables.

And here is the working example based on your code. Producer produces numbers from 5 down to 0, consumer consumes them. Please remember, this will work on x86 only due to weaker ordering on other architectures:

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

volatile int P = 0;
volatile int C = 0;
volatile int value = 0;

void produce(int v)
{
    value = v;
    P++;
}
int consume()
{
    int v = value;
    C++;
    return v;
}

void waitForConsumer()
{
    while (C != P)
        ;
}

void waitForProducer()
{
    while (C == P)
        ;
}


void *producer(void *n)
{
    int i = *(int *)n;
    while (1) {
        waitForConsumer();
        printf("Producing %d\n", i);
        produce(i);
        i--;
        if (i < 0) {
            printf("Exiting: producer\n");
            pthread_exit(0);
        }
    }
}

void *consumer(void *n)
{
    while (1) {
        waitForProducer();
        int v = consume();
        printf("Consumed %d\n", v);
        if (v == 0) {
            printf("Exiting: consumer\n");
            pthread_exit(0);
        }
    }
}

int main(int argc, char *argv[])
{

    int pro = 5;

    pthread_t pid, cid;
    pthread_attr_t attr;

    pthread_attr_init(&attr);

    pthread_create(&pid, &attr, producer, (void *)&pro);
    pthread_create(&cid, &attr, consumer, NULL);

    pthread_join(pid, NULL);
    pthread_join(cid, NULL);
}

Produces the following result:

$ ./a.out
Producing 5
Producing 4
Consumed 5
Consumed 4
Producing 3
Producing 2
Consumed 3
Consumed 2
Producing 1
Producing 0
Exiting: producer
Consumed 1
Consumed 0
Exiting: consumer

For more information, I really recommend Herb Sutter's presentation called atomic<> Weapons, which is quite long, but has everything you need to know about ordering and atomics.

Despite the code listed above will work OK on x86, I really encourage you to watch the presentation above and use builtin atomics, like __atomic_load_n(), which will generate the correct assembly code on any platform.

Andriy Berestovskyy
  • 8,059
  • 3
  • 17
  • 33
0

Create new threads for producer and consumer each i.e all producers and consumers have their own threads.

Prajval M
  • 2,298
  • 11
  • 32